diff --git a/API.md b/API.md index 20514c2f..3d735f60 100644 --- a/API.md +++ b/API.md @@ -24,6 +24,7 @@ This document describes the available endpoints and their usage. All endpoints r - [DELETE /api/history](#delete-apihistory) - [POST /api/history/{id}](#post-apihistoryid) - [GET /api/history/{id}](#get-apihistoryid) + - [POST /api/history/{id}/rename](#post-apihistoryidrename) - [GET /api/history](#get-apihistory) - [GET /api/history/live](#get-apihistorylive) - [POST /api/history/start](#post-apihistorystart) @@ -55,6 +56,8 @@ This document describes the available endpoints and their usage. All endpoints r - [GET /api/player/m3u8/{mode}/{file:.\*}.m3u8](#get-apiplayerm3u8modefilem3u8) - [GET /api/player/segments/{segment}/{file:.\*}.ts](#get-apiplayersegmentssegmentfilets) - [GET /api/player/subtitle/{file:.\*}.vtt](#get-apiplayersubtitlefilevtt) + - [GET /api/player/subtitles/manifest/{file:.\*}](#get-apiplayersubtitlesmanifestfile) + - [GET /api/player/subtitles/{source\_format}/{file:.\*}](#get-apiplayersubtitlessource_formatfile) - [GET /api/thumbnail](#get-apithumbnail) - [GET /api/file/ffprobe/{file:.\*}](#get-apifileffprobefile) - [GET /api/file/info/{file:.\*}](#get-apifileinfofile) @@ -98,6 +101,7 @@ This document describes the available endpoints and their usage. All endpoints r - [GET /api/system/configuration](#get-apisystemconfiguration) - [GET /api/system/limits](#get-apisystemlimits) - [POST /api/system/terminal](#post-apisystemterminal) + - [GET /api/system/terminal](#get-apisystemterminal) - [GET /api/system/terminal/active](#get-apisystemterminalactive) - [GET /api/system/terminal/{session\_id}](#get-apisystemterminalsession_id) - [DELETE /api/system/terminal/{session\_id}](#delete-apisystemterminalsession_id) @@ -529,6 +533,41 @@ or an error: --- +### POST /api/history/{id}/rename +**Purpose**: Rename a downloaded history file and its sidecars. + +**Path Parameter**: +- `id` = Unique history item ID. + +**Body**: +```json +{ + "new_name": "renamed_video.mp4" +} +``` + +**Response**: +```json +{ + "_id": "", + "title": "Video Title", + "url": "https://youtube.com/watch?v=...", + .... +} +``` + +**Error Responses**: +- `400 Bad Request` if `id` or `new_name` is missing, or the item has no downloaded file. +- `404 Not Found` if the item does not exist. +- `409 Conflict` if the rename destination already exists. +- `500 Internal Server Error` if the filesystem rename fails unexpectedly. + +**Notes**: +- Uses the same file rename behavior as the file browser actions. +- Renames matching sidecar files together with the main media file. + +--- + ### GET /api/history **Purpose**: Returns the download queue and/or download history with optional pagination support. @@ -1550,6 +1589,51 @@ Binary TS data (`Content-Type: video/mpegts`). --- +### GET /api/player/subtitles/manifest/{file:.*} +**Purpose**: Returns subtitle track metadata for a local media file. + +**Path Parameter**: +- `file` = Relative path of the media file within the `download_path`. + +**Response**: +```json +{ + "subtitles": [ + { + "lang": "en", + "name": "VTT (0) - en", + "source_format": "vtt", + "delivery_format": "vtt", + "renderer": "native", + "url": "/api/player/subtitles/vtt/path/to/video.en.vtt" + }, + { + "lang": "en", + "name": "ASS (1) - en", + "source_format": "ass", + "delivery_format": "ass", + "renderer": "assjs", + "url": "/api/player/subtitles/ass/path/to/video.en.ass" + } + ] +} +``` + +--- + +### GET /api/player/subtitles/{source_format}/{file:.*} +**Purpose**: Delivers a subtitle file using its preferred playback format. + +**Path Parameters**: +- `source_format` = `vtt`, `srt`, or `ass`. +- `file` = Relative path of the subtitle file. + +**Response**: +- `text/vtt; charset=UTF-8` for `vtt` and `srt` sources. +- `text/x-ssa; charset=UTF-8` for `ass` sources. + +--- + ### GET /api/thumbnail **Purpose**: Proxy/fetch a remote thumbnail image. @@ -1593,21 +1677,16 @@ Binary image data with the appropriate `Content-Type`. }, "mimetype": "video/mp4", "sidecar": { - "subtitles": [ + "subtitle": [ { - "file": "filename.xxx.vtt", + "file": "filename.xxx.ass", "lang": "xxx", - "name": "VTT 0 - XXX|end", + "name": "ASS (0) - xxx" }, ... - } ], - "video": [], - "audio": [], "image": [], - "text": [], - "metadata": [], - ... + "text": [] } } ``` @@ -2579,6 +2658,36 @@ or an error: --- +### GET /api/system/terminal +**Purpose**: List recent terminal sessions. + +**Response**: +```json +{ + "items": [ + { + "session_id": "3a8c5f7e2d3b4a8f9c0d1e2f3a4b5c6d", + "command": "--help", + "status": "completed", + "created_at": 1713000000.0, + "started_at": 1713000000.0, + "finished_at": 1713000004.0, + "expires_at": 1713086404.0, + "available_until": 1713086404.0, + "exit_code": 0, + "last_sequence": 15 + } + ] +} +``` + +**Notes**: +- Expired sessions are removed automatically later. + +- `403 Forbidden` if console is disabled. + +--- + ### GET /api/system/terminal/active **Purpose**: Return the currently active terminal session metadata, or `null` if no session is active. @@ -2606,7 +2715,7 @@ or --- ### GET /api/system/terminal/{session_id} -**Purpose**: Return metadata for a specific terminal session while it is active or still within the replay/drain window. +**Purpose**: Return metadata for a specific terminal session. **Response**: ```json @@ -2617,7 +2726,7 @@ or "created_at": 1713000000.0, "started_at": 1713000000.0, "finished_at": 1713000004.0, - "expires_at": 1713000034.0, + "expires_at": 1713086404.0, "exit_code": 0, "last_sequence": 15 } @@ -2641,8 +2750,7 @@ or **Notes**: - This only applies to the currently active session. -- The client should stay attached to the stream to receive the final `close` event and refreshed terminal status. -- Cancelled sessions finalize as `interrupted` and remain replayable until the drain window expires. +- Cancelled sessions are marked as `interrupted`. - `403 Forbidden` if console is disabled. - `404 Not Found` if the session does not exist or has already expired. @@ -2674,10 +2782,6 @@ If both `since` and `Last-Event-ID` are present, the larger value is used. { "exitcode": 0 } ``` -**Notes**: -- Replay/restore works while the session is still running or until the finished session expires. -- Finished sessions are removed lazily after the transcript drain window elapses. - - `403 Forbidden` if console is disabled. - `400 Bad Request` if the replay cursor is invalid. - `404 Not Found` if the session does not exist or has already expired. diff --git a/FAQ.md b/FAQ.md index c4f53db4..6115c1fc 100644 --- a/FAQ.md +++ b/FAQ.md @@ -167,6 +167,28 @@ YTP_YTDLP_VERSION=2025.07.21 or master or nightly Then restart the container to apply the changes. +# Custom output template placeholders + +YTPTube supports custom `ytp_*` placeholders in `yt-dlp` output template via the following syntax `%(ytp_*:)s`. + +## Currently available extra placeholders are: + +- `ytp_random`: random mixed letters and digits, + - `N` A number is required to specify the length of the random string, for example `%(ytp_random:8)s` will generate a random string of 8 characters. + - if the args followed by `:s` it will generate random letters only, if followed by `:d` it will generate random digits only. + +## Examples of the custom placeholders in action: + +- Template: `%(title)s [%(ytp_random:8)s].%(ext)s` + - Example result: `My Video [A7k2Pq9Z].mp4` +- Template: `%(uploader)s/%(ytp_random:6:d)s - %(title)s.%(ext)s` + - Example result: `MyChannel/483920 - My Video.mp4` +- Template: `%(playlist)s/%(ytp_random:10:s)s/%(title)s.%(ext)s` + - Example result: `Favorites/QwErTyUiOp/My Video.mp4` + +> [!NOTE] +> `%(ytp_` placeholders are a YTPTube extension and not avaliable via console or directly via yt-dlp. + # How can I monitor sites without RSS feeds? YTPTube includes a **generic task handler** that turns JSON definitions into site-specific scrapers. You can use it diff --git a/app/features/conditions/tests/test_conditions.py b/app/features/conditions/tests/test_conditions.py index f72ec622..0586404f 100644 --- a/app/features/conditions/tests/test_conditions.py +++ b/app/features/conditions/tests/test_conditions.py @@ -53,7 +53,7 @@ class TestAllowInternalUrlsScope: Config._reset_singleton() @pytest.mark.asyncio - async def test_conditions_test_rejects_internal_url_when_disallowed(self) -> None: + async def test_rejects_internal_url(self) -> None: config = Config.get_instance() config.allow_internal_urls = False encoder = Encoder() diff --git a/app/features/presets/tests/test_presets_repository.py b/app/features/presets/tests/test_presets_repository.py index 1dfe6746..5b26de4a 100644 --- a/app/features/presets/tests/test_presets_repository.py +++ b/app/features/presets/tests/test_presets_repository.py @@ -99,7 +99,7 @@ class TestPresetsRepository: assert total_pages == 1, "Should compute pages from the filtered total" @pytest.mark.asyncio - async def test_list_paginated_supports_multiple_sort_fields(self, repo): + async def test_list_paginated_multi_sort(self, repo): await repo.create({"name": "Charlie", "priority": 2}) await repo.create({"name": "Alpha", "priority": 1}) await repo.create({"name": "Bravo", "priority": 1}) @@ -113,24 +113,24 @@ class TestPresetsRepository: ], "Should support multiple sort fields and directions" @pytest.mark.asyncio - async def test_list_paginated_rejects_invalid_sort_field(self, repo): + async def test_list_paginated_bad_sort(self, repo): with pytest.raises(ValueError, match="sort must use supported fields"): await repo.list_paginated(page=1, per_page=10, sort="cli", order="asc") @pytest.mark.asyncio - async def test_list_paginated_rejects_invalid_sort_direction(self, repo): + async def test_list_paginated_bad_order(self, repo): with pytest.raises(ValueError, match="order must be 'asc' or 'desc'"): await repo.list_paginated(page=1, per_page=10, sort="name", order="sideways") @pytest.mark.asyncio - async def test_list_paginated_rejects_mismatched_sort_and_order_lengths(self, repo): + async def test_list_paginated_mismatched_order(self, repo): with pytest.raises(ValueError, match="order must provide one direction or match the number of sort fields"): await repo.list_paginated(page=1, per_page=10, sort="priority,name", order="asc,desc,asc") @pytest.mark.asyncio class TestPresetRoutes: - async def test_list_route_supports_sort_params(self, repo): + async def test_list_route_sort(self, repo): await repo.create({"name": "Alpha", "priority": 1}) await repo.create({"name": "Bravo", "priority": 1}) await repo.create({"name": "Charlie", "priority": 2}) @@ -144,7 +144,7 @@ class TestPresetRoutes: assert response.status == web.HTTPOk.status_code, "Should return 200 for valid sorting" assert [item["name"] for item in payload["items"]] == ["bravo", "alpha", "charlie"], "Should sort response" - async def test_list_route_rejects_invalid_sort_field(self, repo): + async def test_list_route_bad_sort(self, repo): request = MagicMock(spec=Request) request.query = {"sort": "cli", "order": "asc"} @@ -154,7 +154,7 @@ class TestPresetRoutes: assert response.status == web.HTTPBadRequest.status_code, "Should reject unsupported sort field" assert "sort" in payload["error"], "Should explain invalid sort field" - async def test_list_route_rejects_invalid_sort_direction(self, repo): + async def test_list_route_bad_order(self, repo): request = MagicMock(spec=Request) request.query = {"sort": "name", "order": "sideways"} @@ -164,7 +164,7 @@ class TestPresetRoutes: assert response.status == web.HTTPBadRequest.status_code, "Should reject unsupported sort direction" assert "order" in payload["error"], "Should explain invalid sort direction" - async def test_list_route_supports_excluding_defaults(self, repo): + async def test_list_route_exclude_defaults(self, repo): await repo.create({"name": "System Default", "default": True, "priority": 10}) await repo.create({"name": "Custom Preset", "priority": 1}) diff --git a/app/features/streaming/library/subtitle.py b/app/features/streaming/library/subtitle.py index b44fe645..7c4f3d61 100644 --- a/app/features/streaming/library/subtitle.py +++ b/app/features/streaming/library/subtitle.py @@ -1,4 +1,7 @@ +from __future__ import annotations + import logging +from dataclasses import dataclass from pathlib import Path import anyio @@ -6,10 +9,37 @@ import pysubs2 from pysubs2.formats.substation import SubstationFormat from pysubs2.time import ms_to_times -from app.library.Utils import ALLOWED_SUBS_EXTENSIONS +from app.library.Utils import ALLOWED_SUBS_EXTENSIONS, get_file_sidecar LOG: logging.Logger = logging.getLogger("player.subtitle") +SOURCE_FORMATS: tuple[str, ...] = ("vtt", "srt", "ass") +DELIVERY_FORMATS: dict[str, str] = { + "vtt": "vtt", + "srt": "vtt", + "ass": "ass", +} +RENDERERS: dict[str, str] = { + "vtt": "native", + "srt": "native", + "ass": "assjs", +} +MEDIA_TYPES: dict[str, str] = { + "vtt": "text/vtt; charset=UTF-8", + "ass": "text/x-ssa; charset=UTF-8", +} +TEXT_ENCODINGS: tuple[str, ...] = ("utf-8-sig", "utf-16", "utf-16-le", "utf-16-be", "cp1252") + + +@dataclass(frozen=True, slots=True) +class SubtitleTrack: + file: Path + lang: str + name: str + source_format: str + delivery_format: str + renderer: str + def ms_to_timestamp(ms: int) -> str: ms = max(0, ms) @@ -22,14 +52,46 @@ SubstationFormat.ms_to_timestamp = ms_to_timestamp class Subtitle: + @staticmethod + def normalize_format(source_format: str) -> str | None: + fmt = source_format.strip().lower().removeprefix(".") + if fmt not in SOURCE_FORMATS: + return None + + return fmt + + async def read_text(self, file: Path) -> str: + async with await anyio.open_file(file, "rb") as f: + subtitle_bytes = await f.read() + + return self.decode_bytes(subtitle_bytes) + + @staticmethod + def decode_bytes(subtitle_bytes: bytes) -> str: + for encoding in TEXT_ENCODINGS: + try: + return subtitle_bytes.decode(encoding) + except UnicodeDecodeError: + continue + + return subtitle_bytes.decode("utf-8", errors="replace") + + def media_type(self, file: Path) -> str: + fmt = self.normalize_format(file.suffix) + if fmt is None: + msg = f"File '{file}' subtitle type is not supported." + raise Exception(msg) + + return MEDIA_TYPES[DELIVERY_FORMATS[fmt]] + async def make(self, file: Path) -> str: - if file.suffix not in ALLOWED_SUBS_EXTENSIONS: + fmt = self.normalize_format(file.suffix) + if fmt is None or file.suffix not in ALLOWED_SUBS_EXTENSIONS: msg: str = f"File '{file}' subtitle type is not supported." raise Exception(msg) - if file.suffix == ".vtt": - async with await anyio.open_file(file) as f: - return await f.read() + if fmt == "vtt": + return await self.read_text(file) subs: pysubs2.SSAFile = pysubs2.load(path=str(file)) @@ -47,3 +109,45 @@ class Subtitle: pass return subs.to_string("vtt") + + async def make_delivery(self, file: Path) -> tuple[str, str]: + fmt = self.normalize_format(file.suffix) + if fmt is None or file.suffix not in ALLOWED_SUBS_EXTENSIONS: + msg: str = f"File '{file}' subtitle type is not supported." + raise Exception(msg) + + if fmt == "ass": + return await self.read_text(file), self.media_type(file) + + return await self.make(file), self.media_type(file) + + +def get_subtitle_tracks(file: Path) -> list[SubtitleTrack]: + sidecars = get_file_sidecar(file).get("subtitle", []) + indexed_tracks: list[tuple[int, SubtitleTrack]] = [] + + for index, item in enumerate(sidecars): + track_file = item.get("file") + if not isinstance(track_file, Path): + continue + + fmt = Subtitle.normalize_format(track_file.suffix) + if fmt is None: + continue + + indexed_tracks.append( + ( + index, + SubtitleTrack( + file=track_file, + lang=str(item.get("lang") or "und"), + name=str(item.get("name") or track_file.name), + source_format=fmt, + delivery_format=DELIVERY_FORMATS[fmt], + renderer=RENDERERS[fmt], + ), + ) + ) + + indexed_tracks.sort(key=lambda item: (SOURCE_FORMATS.index(item[1].source_format), item[0])) + return [track for _, track in indexed_tracks] diff --git a/app/features/streaming/router.py b/app/features/streaming/router.py index 84b4970a..5affd517 100644 --- a/app/features/streaming/router.py +++ b/app/features/streaming/router.py @@ -9,7 +9,7 @@ from aiohttp.web import Request, Response from app.features.streaming.library.m3u8 import M3u8 from app.features.streaming.library.playlist import Playlist from app.features.streaming.library.segments import Segments -from app.features.streaming.library.subtitle import Subtitle +from app.features.streaming.library.subtitle import Subtitle, get_subtitle_tracks from app.features.streaming.types import StreamingError from app.library.config import Config from app.library.router import route @@ -277,3 +277,134 @@ async def subtitles_get(request: Request, config: Config, app: web.Application) }, status=web.HTTPOk.status_code, ) + + +@route("GET", "api/player/subtitles/manifest/{file:.*}", "subtitles_manifest_get") +async def subtitles_manifest_get(request: Request, config: Config, app: web.Application) -> Response: + """ + Get subtitle track metadata for a media file. + + Args: + request (Request): The request object. + config (Config): The configuration instance. + app (web.Application): The aiohttp application instance. + + Returns: + Response: The response object. + + """ + file: str = request.match_info.get("file") + + if not file: + return web.json_response(data={"error": "file is required"}, status=web.HTTPBadRequest.status_code) + + realFile, status = get_file(download_path=config.download_path, file=file) + if web.HTTPFound.status_code == status: + return Response( + status=status, + headers={ + "Location": str( + app.router["subtitles_manifest_get"].url_for( + file=str(realFile).replace(config.download_path, "").strip("/") + ) + ), + }, + ) + + if web.HTTPNotFound.status_code == status: + return web.json_response(data={"error": f"File '{file}' does not exist."}, status=status) + + tracks = [ + { + "lang": track.lang, + "name": track.name, + "source_format": track.source_format, + "delivery_format": track.delivery_format, + "renderer": track.renderer, + "url": str( + app.router["subtitles_track_get"].url_for( + source_format=track.source_format, + file=str(track.file).replace(config.download_path, "").strip("/"), + ) + ), + } + for track in get_subtitle_tracks(realFile) + ] + + return web.json_response(data={"subtitles": tracks}, status=web.HTTPOk.status_code) + + +@route("GET", "api/player/subtitles/{source_format}/{file:.*}", "subtitles_track_get") +async def subtitles_track_get(request: Request, config: Config, app: web.Application) -> Response: + """ + Get a subtitle file using its preferred delivery format. + + Args: + request (Request): The request object. + config (Config): The configuration instance. + app (web.Application): The aiohttp application instance. + + Returns: + Response: The response object. + + """ + file: str = request.match_info.get("file") + source_format: str | None = request.match_info.get("source_format") + + if not file: + return web.json_response(data={"error": "file is required"}, status=web.HTTPBadRequest.status_code) + + fmt = Subtitle.normalize_format(source_format or "") + if fmt is None: + return web.json_response( + data={"error": "Only vtt, srt, and ass subtitle formats are supported."}, + status=web.HTTPBadRequest.status_code, + ) + + realFile, status = get_file(download_path=config.download_path, file=file) + if web.HTTPFound.status_code == status: + return Response( + status=status, + headers={ + "Location": str( + app.router["subtitles_track_get"].url_for( + source_format=fmt, + file=str(realFile).replace(config.download_path, "").strip("/"), + ) + ), + }, + ) + + if web.HTTPNotFound.status_code == status: + return web.json_response(data={"error": f"File '{file}' does not exist."}, status=status) + + if Subtitle.normalize_format(realFile.suffix) != fmt: + return web.json_response( + data={"error": f"Subtitle file '{file}' does not match requested source format '{fmt}'."}, + status=web.HTTPBadRequest.status_code, + ) + + mtime = realFile.stat().st_mtime + + if request.if_modified_since and request.if_modified_since.timestamp() == mtime: + lastMod = time.strftime("%a, %d %b %Y %H:%M:%S GMT", datetime.fromtimestamp(mtime, tz=UTC).timetuple()) + return web.Response(status=web.HTTPNotModified.status_code, headers={"Last-Modified": lastMod}) + + body, content_type = await Subtitle().make_delivery(file=realFile) + return web.Response( + body=body, + headers={ + "Content-Type": content_type, + "X-Accel-Buffering": "no", + "Access-Control-Allow-Origin": "*", + "Pragma": "public", + "Cache-Control": f"public, max-age={time.time() + 31536000}", + "Last-Modified": time.strftime( + "%a, %d %b %Y %H:%M:%S GMT", datetime.fromtimestamp(mtime, tz=UTC).timetuple() + ), + "Expires": time.strftime( + "%a, %d %b %Y %H:%M:%S GMT", datetime.fromtimestamp(time.time() + 31536000, tz=UTC).timetuple() + ), + }, + status=web.HTTPOk.status_code, + ) diff --git a/app/features/streaming/tests/test_ffprobe.py b/app/features/streaming/tests/test_ffprobe.py index 621ac12b..5982386b 100644 --- a/app/features/streaming/tests/test_ffprobe.py +++ b/app/features/streaming/tests/test_ffprobe.py @@ -84,13 +84,11 @@ class TestFFProbe: # First call should execute the function result1 = await ffprobe(str(self.test_file)) - assert result1 is not None assert isinstance(result1.metadata, dict) first_call_count = call_count # Second call with same argument should use cached result result2 = await ffprobe(str(self.test_file)) - assert result2 is not None assert isinstance(result2.metadata, dict) assert call_count == first_call_count, ( @@ -119,7 +117,7 @@ class TestFFProbe: # Test with Path object result = await ffprobe(self.test_file) - assert result is not None + assert result.metadata == {"duration": "10.0"} def test_ffprobe_result_properties(self): """Test FFProbeResult object properties.""" diff --git a/app/features/streaming/tests/test_m3u8.py b/app/features/streaming/tests/test_m3u8.py index 2e97da56..12f1d838 100644 --- a/app/features/streaming/tests/test_m3u8.py +++ b/app/features/streaming/tests/test_m3u8.py @@ -64,7 +64,7 @@ async def test_make_stream_basic_ok_codecs(tmp_path: Path, monkeypatch: pytest.M @pytest.mark.asyncio -async def test_make_stream_transcode_flags_and_remainder(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +async def test_make_stream_transcode_flags(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: base = tmp_path / "dl" base.mkdir() media = base / "v.mp4" @@ -107,7 +107,7 @@ async def test_make_stream_transcode_flags_and_remainder(tmp_path: Path, monkeyp @pytest.mark.asyncio -async def test_make_stream_raises_without_duration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +async def test_make_stream_no_duration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: base = tmp_path / "dl" base.mkdir() media = base / "v.mp4" diff --git a/app/features/streaming/tests/test_playlist.py b/app/features/streaming/tests/test_playlist.py index f69025ea..90decebd 100644 --- a/app/features/streaming/tests/test_playlist.py +++ b/app/features/streaming/tests/test_playlist.py @@ -99,7 +99,7 @@ async def test_make_playlist_with_subtitles(tmp_path: Path, monkeypatch: pytest. @pytest.mark.asyncio -async def test_make_playlist_raises_without_duration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +async def test_make_playlist_no_duration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: base = tmp_path / "downloads" base.mkdir() media = base / "file.mp4" diff --git a/app/features/streaming/tests/test_segments.py b/app/features/streaming/tests/test_segments.py index 57c30e5a..33ec0e03 100644 --- a/app/features/streaming/tests/test_segments.py +++ b/app/features/streaming/tests/test_segments.py @@ -172,7 +172,7 @@ class _FakeProcFail(_FakeProc): @pytest.mark.asyncio -async def test_build_ffmpeg_args_no_dri_falls_back_to_software(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +async def test_build_ffmpeg_args_no_dri(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: media = tmp_path / "file.mp4" media.write_bytes(b"data") @@ -219,7 +219,7 @@ async def test_build_ffmpeg_args_no_dri_falls_back_to_software(tmp_path: Path, m @pytest.mark.asyncio -async def test_stream_gpu_failure_falls_back_to_software( +async def test_stream_gpu_fallback( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: async def fake_ffprobe(_file: Path): diff --git a/app/features/streaming/tests/test_subtitle.py b/app/features/streaming/tests/test_subtitle.py index 8e06d03b..95426a22 100644 --- a/app/features/streaming/tests/test_subtitle.py +++ b/app/features/streaming/tests/test_subtitle.py @@ -4,7 +4,7 @@ from unittest.mock import patch import pytest -from app.features.streaming.library.subtitle import Subtitle, ms_to_timestamp +from app.features.streaming.library.subtitle import Subtitle, get_subtitle_tracks, ms_to_timestamp class TestMsToTimestamp: @@ -24,12 +24,12 @@ class TestMsToTimestamp: @pytest.mark.asyncio async def test_make_unsupported_extension(tmp_path: Path) -> None: - srt = tmp_path / "sub.txt" - srt.write_text("not a subtitle") + file = tmp_path / "sub.txt" + file.write_text("not a subtitle") - sub = Subtitle() + subtitle = Subtitle() with pytest.raises(Exception, match="subtitle type is not supported"): - await sub.make(srt) + await subtitle.make(file) @pytest.mark.asyncio @@ -38,11 +38,51 @@ async def test_make_vtt_reads_file(tmp_path: Path) -> None: content = "WEBVTT\n\n00:00:00.00 --> 00:00:01.00\nHello" vtt.write_text(content) - sub = Subtitle() - out = await sub.make(vtt) + subtitle = Subtitle() + out = await subtitle.make(vtt) assert out == content +@pytest.mark.asyncio +async def test_make_delivery_ass(tmp_path: Path) -> None: + ass = tmp_path / "file.ass" + content = "[Script Info]\nTitle: Demo\n" + ass.write_text(content, encoding="utf-8") + + subtitle = Subtitle() + out, media_type = await subtitle.make_delivery(ass) + assert out == content + assert media_type == "text/x-ssa; charset=UTF-8" + + +def test_tracks_order(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + media = tmp_path / "video.mkv" + media.write_text("x", encoding="utf-8") + ass_file = tmp_path / "video.ass" + ass_file.write_text("ass", encoding="utf-8") + vtt_file = tmp_path / "video.vtt" + vtt_file.write_text("WEBVTT\n\n", encoding="utf-8") + srt_file = tmp_path / "video.en.srt" + srt_file.write_text("1\n00:00:00,000 --> 00:00:01,000\nHello\n", encoding="utf-8") + + monkeypatch.setattr( + "app.features.streaming.library.subtitle.get_file_sidecar", + lambda _file: { + "subtitle": [ + {"file": ass_file, "lang": "en", "name": "ASS"}, + {"file": srt_file, "lang": "en", "name": "SRT"}, + {"file": vtt_file, "lang": "en", "name": "VTT"}, + ] + }, + ) + + tracks = get_subtitle_tracks(media) + + assert [track.source_format for track in tracks] == ["vtt", "srt", "ass"] + assert [track.delivery_format for track in tracks] == ["vtt", "vtt", "ass"] + assert [track.renderer for track in tracks] == ["native", "native", "assjs"] + + class _DummySubs: def __init__(self, events): self.events = events @@ -61,13 +101,13 @@ async def test_make_no_events_raises(tmp_path: Path) -> None: with patch("app.features.streaming.library.subtitle.pysubs2.load") as mock_load: mock_load.return_value = _DummySubs(events=[]) - sub = Subtitle() + subtitle = Subtitle() with pytest.raises(Exception, match="No subtitle events were found"): - await sub.make(srt) + await subtitle.make(srt) @pytest.mark.asyncio -async def test_make_single_event_returns_vtt(tmp_path: Path) -> None: +async def test_make_single_vtt(tmp_path: Path) -> None: srt = tmp_path / "sub.ass" srt.write_text("dummy") @@ -75,14 +115,14 @@ async def test_make_single_event_returns_vtt(tmp_path: Path) -> None: d = _DummySubs(events=[single]) with patch("app.features.streaming.library.subtitle.pysubs2.load", return_value=d): - sub = Subtitle() - out = await sub.make(srt) + subtitle = Subtitle() + out = await subtitle.make(srt) assert out == "OUT" assert d.snapshot == [1000], "Snapshot should contain the single event" @pytest.mark.asyncio -async def test_make_two_events_pop_first_when_ends_equal(tmp_path: Path) -> None: +async def test_make_two_events_same_end(tmp_path: Path) -> None: srt = tmp_path / "sub.srt" srt.write_text("dummy") @@ -91,14 +131,14 @@ async def test_make_two_events_pop_first_when_ends_equal(tmp_path: Path) -> None d = _DummySubs(events=[e1, e2]) with patch("app.features.streaming.library.subtitle.pysubs2.load", return_value=d): - sub = Subtitle() - out = await sub.make(srt) + subtitle = Subtitle() + out = await subtitle.make(srt) assert out == "OUT" assert d.snapshot == [5000], "Since ends are equal, first should be popped => only last remains" @pytest.mark.asyncio -async def test_make_two_events_no_pop_when_different(tmp_path: Path) -> None: +async def test_make_two_events_keep_both(tmp_path: Path) -> None: srt = tmp_path / "sub.srt" srt.write_text("dummy") @@ -107,7 +147,7 @@ async def test_make_two_events_no_pop_when_different(tmp_path: Path) -> None: d = _DummySubs(events=[e1, e2]) with patch("app.features.streaming.library.subtitle.pysubs2.load", return_value=d): - sub = Subtitle() - out = await sub.make(srt) + subtitle = Subtitle() + out = await subtitle.make(srt) assert out == "OUT" assert d.snapshot == [5000, 6000], "Both remain since ends differ" diff --git a/app/features/streaming/tests/test_subtitle_routes.py b/app/features/streaming/tests/test_subtitle_routes.py new file mode 100644 index 00000000..4fc3248b --- /dev/null +++ b/app/features/streaming/tests/test_subtitle_routes.py @@ -0,0 +1,132 @@ +from pathlib import Path +from types import SimpleNamespace + +import pytest +from aiohttp import web +from aiohttp.test_utils import make_mocked_request + +from app.features.streaming import router +from app.library.config import Config + + +class _DummyRoute: + def __init__(self, pattern: str) -> None: + self.pattern = pattern + + def url_for(self, **params: str) -> str: + path = self.pattern + for key, value in params.items(): + path = path.replace(f"{{{key}:.*}}", value) + path = path.replace(f"{{{key}}}", value) + return path + + +class _DummyRouter(dict[str, _DummyRoute]): + def __init__(self) -> None: + super().__init__( + { + "subtitles_manifest_get": _DummyRoute("/api/player/subtitles/manifest/{file:.*}"), + "subtitles_track_get": _DummyRoute("/api/player/subtitles/{source_format}/{file:.*}"), + "subtitles_get": _DummyRoute("/api/player/subtitle/{file:.*}.vtt"), + } + ) + + +def _make_request(path: str, *, match_info: dict[str, str]) -> web.Request: + return make_mocked_request("GET", path, app=SimpleNamespace(router=_DummyRouter()), match_info=match_info) + + +@pytest.mark.asyncio +async def test_subtitles_manifest_order(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + config = Config.get_instance() + config.download_path = str(tmp_path) + + media = tmp_path / "video.mp4" + media.write_text("x", encoding="utf-8") + + monkeypatch.setattr( + "app.features.streaming.router.get_file", + lambda **_kwargs: (media, web.HTTPOk.status_code), + ) + monkeypatch.setattr( + "app.features.streaming.router.get_subtitle_tracks", + lambda _file: [ + SimpleNamespace( + lang="en", + name="English VTT", + source_format="vtt", + delivery_format="vtt", + renderer="native", + file=tmp_path / "video.vtt", + ), + SimpleNamespace( + lang="en", + name="English ASS", + source_format="ass", + delivery_format="ass", + renderer="assjs", + file=tmp_path / "video.ass", + ), + ], + ) + + req = _make_request( + "/api/player/subtitles/manifest/video.mp4", + match_info={"file": "video.mp4"}, + ) + response = await router.subtitles_manifest_get(req, config, req.app) + + assert response.status == web.HTTPOk.status_code + body = response.text + assert '"source_format": "vtt"' in body + assert '"renderer": "native"' in body + assert '"source_format": "ass"' in body + assert '"renderer": "assjs"' in body + assert body.index('"source_format": "vtt"') < body.index('"source_format": "ass"') + + +@pytest.mark.asyncio +async def test_subtitles_track_get_ass(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + config = Config.get_instance() + config.download_path = str(tmp_path) + + subtitle = tmp_path / "video.ass" + subtitle.write_text("[Script Info]\nTitle: Demo\n", encoding="utf-8") + + monkeypatch.setattr( + "app.features.streaming.router.get_file", + lambda **_kwargs: (subtitle, web.HTTPOk.status_code), + ) + + req = _make_request( + "/api/player/subtitles/ass/video.ass", + match_info={"source_format": "ass", "file": "video.ass"}, + ) + response = await router.subtitles_track_get(req, config, req.app) + + assert response.status == web.HTTPOk.status_code + assert response.text == "[Script Info]\nTitle: Demo\n" + assert response.headers["Content-Type"] == "text/x-ssa; charset=UTF-8" + + +@pytest.mark.asyncio +async def test_subtitles_track_bad_format(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + config = Config.get_instance() + config.download_path = str(tmp_path) + + subtitle = tmp_path / "video.ass" + subtitle.write_text("[Script Info]\nTitle: Demo\n", encoding="utf-8") + + monkeypatch.setattr( + "app.features.streaming.router.get_file", + lambda **_kwargs: (subtitle, web.HTTPOk.status_code), + ) + + req = _make_request( + "/api/player/subtitles/vtt/video.ass", + match_info={"source_format": "vtt", "file": "video.ass"}, + ) + response = await router.subtitles_track_get(req, config, req.app) + + assert response.status == web.HTTPBadRequest.status_code + assert b"does not match requested source format" in response.body diff --git a/app/features/tasks/definitions/tests/test_generic_task_handler.py b/app/features/tasks/definitions/tests/test_generic_task_handler.py index eda8392e..d06b3d67 100644 --- a/app/features/tasks/definitions/tests/test_generic_task_handler.py +++ b/app/features/tasks/definitions/tests/test_generic_task_handler.py @@ -21,7 +21,7 @@ def reset_generic_handler(monkeypatch): monkeypatch.setattr(GenericTaskHandler, "_sources_mtime", {}) -def test_build_task_definition_parses_valid_payload(): +def test_build_def_payload(): definition = TaskDefinition( id=1, name="example", @@ -40,16 +40,12 @@ def test_build_task_definition_parses_valid_payload(): ), ) - assert definition is not None, "TaskDefinition should be created" - assert "example" == definition.name, "Name should match" - assert "https://example.com/articles/*" in definition.match_url, "Match URL should be in list" - assert "link" in definition.definition.parse, "Parse should contain link field" - assert ".article a.link::attr(href)" == definition.definition.parse["link"]["expression"], ( - "Link expression should match" - ) + assert definition.name == "example" + assert definition.match_url == ["https://example.com/articles/*"] + assert definition.definition.parse["link"]["expression"] == ".article a.link::attr(href)" -def test_build_task_definition_handles_container(): +def test_build_def_container(): definition = TaskDefinition( id=2, name="container", @@ -73,13 +69,11 @@ def test_build_task_definition_handles_container(): ), ) - assert definition is not None, "TaskDefinition should be created" - assert "items" in definition.definition.parse, "Parse should contain items container" - assert ".cards .card" == definition.definition.parse["items"]["selector"], "Items selector should match" - assert "link" in definition.definition.parse["items"]["fields"], "Items fields should contain link" + assert definition.definition.parse["items"]["selector"] == ".cards .card" + assert definition.definition.parse["items"]["fields"]["link"]["attribute"] == "href" -def test_build_task_definition_handles_json(): +def test_build_def_json(): definition = TaskDefinition( id=3, name="json-def", @@ -104,16 +98,12 @@ def test_build_task_definition_handles_json(): ), ) - assert definition is not None, "TaskDefinition should be created" - assert "json" == definition.definition.response.type, "Response type should be json" - assert "items" in definition.definition.parse, "Parse should contain items container" - assert "jsonpath" == definition.definition.parse["items"]["type"], "Items type should be jsonpath" - assert "jsonpath" == definition.definition.parse["items"]["fields"]["link"]["type"], ( - "Link field type should be jsonpath" - ) + assert definition.definition.response.type == "json" + assert definition.definition.parse["items"]["type"] == "jsonpath" + assert definition.definition.parse["items"]["fields"]["link"]["type"] == "jsonpath" -def test_parse_items_extracts_values(): +def test_parse_items_basic(): definition = TaskDefinition( id=4, name="example", @@ -146,14 +136,16 @@ def test_parse_items_extracts_values(): items = GenericTaskHandler._parse_items(definition, html, "https://example.com/base/") - assert 2 == len(items), "Should extract 2 items" - assert "https://example.com/article-101" == items[0]["link"], "First item link should be absolute URL" - assert "First Title" == items[0]["title"], "First item title should match" - assert "101" == items[0]["id"], "First item id should match" - assert "https://example.com/article-102" == items[1]["link"], "Second item link should match" + assert len(items) == 2 + assert items[0] == { + "link": "https://example.com/article-101", + "title": "First Title", + "id": "101", + } + assert items[1]["link"] == "https://example.com/article-102" -def test_parse_items_handles_nested_card_layout(): +def test_parse_items_cards(): definition = TaskDefinition( id=5, name="nested", @@ -234,19 +226,21 @@ def test_parse_items_handles_nested_card_layout(): items = GenericTaskHandler._parse_items(definition, html, "https://example.com") - assert 2 == len(items), "Should extract 2 items" - assert "https://example.com/poems/view/111" == items[0]["link"], "First item link should match" - assert "First Poem" == items[0]["title"], "First item title should match" - assert "Poet Alpha" == items[0]["poet"], "First item poet should match" - assert "Category One" == items[0]["category"], "First item category should match" - - assert "https://example.com/poems/view/222" == items[1]["link"], "Second item link should match" - assert "Second Poem" == items[1]["title"], "Second item title should match" - assert "Poet Beta" == items[1]["poet"], "Second item poet should match" - assert "category" not in items[1], "Second item should not have category" + assert len(items) == 2 + assert items[0] == { + "link": "https://example.com/poems/view/111", + "title": "First Poem", + "poet": "Poet Alpha", + "category": "Category One", + } + assert items[1] == { + "link": "https://example.com/poems/view/222", + "title": "Second Poem", + "poet": "Poet Beta", + } -def test_parse_items_handles_json_container(): +def test_parse_items_json(): definition = TaskDefinition( id=6, name="json", @@ -287,14 +281,10 @@ def test_parse_items_handles_json_container(): json_data=payload, ) - assert 2 == len(items), "Should extract 2 items (third missing link)" - assert "https://example.com/video/1" == items[0]["link"], "First item link should be absolute" - assert "First" == items[0]["title"], "First item title should match" - assert "1" == items[0]["id"], "First item id should match" - - assert "https://example.com/video/2" == items[1]["link"], "Second item link should match" - assert "Second" == items[1]["title"], "Second item title should match" - assert "2" == items[1]["id"], "Second item id should match" + assert items == [ + {"link": "https://example.com/video/1", "title": "First", "id": "1"}, + {"link": "https://example.com/video/2", "title": "Second", "id": "2"}, + ] @pytest.mark.asyncio @@ -345,14 +335,14 @@ async def test_generic_task_handler_inspect(monkeypatch): task = HandleTask(id=1, name="Inspect", url="https://example.com/api") result: TaskResult | TaskFailure = await GenericTaskHandler.extract(task) - assert isinstance(result, TaskResult), "Result should be TaskResult" - assert 1 == len(result.items), "Should have 1 item" + assert isinstance(result, TaskResult) + assert len(result.items) == 1 item = result.items[0] - assert "https://example.com/video/1" == item.url, "Item URL should match" - assert "First" == item.title, "Item title should match" + assert item.url == "https://example.com/video/1" + assert item.title == "First" -def test_parse_items_handles_json_top_level_list(): +def test_parse_items_json_list(): definition = TaskDefinition( id=8, name="json-list", @@ -389,8 +379,7 @@ def test_parse_items_handles_json_top_level_list(): json_data=payload, ) - assert 2 == len(items), "Should extract 2 items" - assert "https://example.com/video/1" == items[0]["link"], "First item link should match" - assert "First" == items[0]["title"], "First item title should match" - assert "https://example.com/video/2" == items[1]["link"], "Second item link should match" - assert "Second" == items[1]["title"], "Second item title should match" + assert items == [ + {"link": "https://example.com/video/1", "title": "First"}, + {"link": "https://example.com/video/2", "title": "Second"}, + ] diff --git a/app/features/tasks/definitions/tests/test_tver_handler.py b/app/features/tasks/definitions/tests/test_tver_handler.py index b2179c1a..d5711728 100644 --- a/app/features/tasks/definitions/tests/test_tver_handler.py +++ b/app/features/tasks/definitions/tests/test_tver_handler.py @@ -142,7 +142,7 @@ async def test_tver_handler_extract(monkeypatch): @pytest.mark.parametrize(("url", "should_match"), TverHandler.tests()) -def test_tver_handler_parse(url: str, should_match: bool): +def test_parse(url: str, should_match: bool): """Test tver URL parsing.""" result = TverHandler.parse(url) if should_match: @@ -153,7 +153,7 @@ def test_tver_handler_parse(url: str, should_match: bool): @pytest.mark.asyncio -async def test_tver_handler_can_handle(): +async def test_can_handle(): """Test tver handler can_handle method.""" task_valid = HandleTask(id=1, name="Test", url="https://tver.jp/series/sr8sb9pnhc", preset="default") task_invalid = HandleTask(id=2, name="Test", url="https://youtube.com/watch?v=123", preset="default") 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/app/features/ytdlp/outtmpl.py b/app/features/ytdlp/outtmpl.py new file mode 100644 index 00000000..974b5477 --- /dev/null +++ b/app/features/ytdlp/outtmpl.py @@ -0,0 +1,113 @@ +from __future__ import annotations + +import re +import secrets +import string +from typing import TYPE_CHECKING, Any + +from yt_dlp.utils._utils import STR_FORMAT_RE_TMPL, STR_FORMAT_TYPES + +if TYPE_CHECKING: + from collections.abc import Callable + +OUTTMPL_RX: re.Pattern[str] = re.compile(STR_FORMAT_RE_TMPL.format("[^)]*", f"[{STR_FORMAT_TYPES}ljhqBUDS]")) +CALL_RX: re.Pattern[str] = re.compile(r"^(?Pytp_[A-Za-z0-9_]+)(?P(?::[A-Za-z0-9_]+)*)(?P.*)$") +OPERATORS: tuple[str, ...] = (",", "&", "|", ">", "+", "-", "*") + + +def random_text(args: tuple[str, ...], _info_dict: dict[str, Any], _state: dict[str, Any]) -> str: + if not args: + msg = "ytp_random requires a length argument. Use %(ytp_random:)s." + raise ValueError(msg) + + if len(args) > 2: + msg = "ytp_random accepts at most 2 arguments: length and optional mode." + raise ValueError(msg) + + try: + length = int(args[0]) + except ValueError as exc: + msg: str = f"ytp_random length must be an integer, got {args[0]!r}." + raise ValueError(msg) from exc + + if length < 1: + msg = "ytp_random length must be greater than 0." + raise ValueError(msg) + + mode = args[1].lower() if len(args) > 1 else "mixed" + if mode in ("mixed", "m"): + alphabet = string.ascii_letters + string.digits + elif mode in ("d", "digit", "digits", "int", "ints", "number", "numbers"): + alphabet = string.digits + elif mode in ("s", "str", "string", "strings", "alpha", "letter", "letters"): + alphabet = string.ascii_letters + else: + msg = f"ytp_random mode must be one of mixed, d, or s. Got {args[1]!r}." + raise ValueError(msg) + + return "".join(secrets.choice(alphabet) for _ in range(length)) + + +CALLS: dict[str, Callable[[tuple[str, ...], dict[str, Any], dict[str, Any]], Any]] = { + "ytp_random": random_text, +} + + +def split_call(key: str) -> tuple[str, tuple[str, ...], str] | None: + if not key.startswith("ytp_"): + return None + + if not (match := CALL_RX.match(key)): + msg = f"Invalid YTPTube output template callable {key!r}." + raise ValueError(msg) + + name: str | Any = match.group("name") + rest: str | Any = match.group("rest") or "" + if rest and rest[0] not in OPERATORS: + msg = f"Invalid YTPTube output template callable {key!r}." + raise ValueError(msg) + + if name not in CALLS: + msg = f"Unsupported YTPTube output template callable {name!r}." + raise ValueError(msg) + + raw_args: str | Any = match.group("args") + args: tuple[str | Any, ...] = tuple(part for part in raw_args.split(":") if part) + return name, args, rest + + +def rewrite_outtmpl( + outtmpl: str, + info_dict: dict[str, Any], + cache: dict[str, Any] | None = None, +) -> tuple[str, dict[str, Any]]: + if "%(ytp_" not in outtmpl: + return outtmpl, info_dict + + state: dict[str, Any] = {} + values: dict[str, Any] = {} if cache is None else cache + fields: dict[str, str] = {} + enriched = dict(info_dict) + + def replace(match: re.Match[str]) -> str: + if not (key := match.group("key")): + return match.group(0) + + parsed = split_call(key) + if not parsed: + return match.group(0) + + name, args, rest = parsed + call_key: str = ":".join((name, *args)) if args else name + + if call_key not in values: + values[call_key] = CALLS[name](args, enriched, state) + + if call_key not in fields: + fields[call_key] = f"__ytptube_outtmpl_{len(fields)}" + + synthetic_key: str = fields[call_key] + enriched[synthetic_key] = values[call_key] + return f"{match.group('prefix')}%({synthetic_key}{rest}){match.group('format')}" + + return OUTTMPL_RX.sub(replace, outtmpl), enriched diff --git a/app/features/ytdlp/tests/test_ytdlp_extractor.py b/app/features/ytdlp/tests/test_ytdlp_extractor.py index 557e0bc7..75381ddd 100644 --- a/app/features/ytdlp/tests/test_ytdlp_extractor.py +++ b/app/features/ytdlp/tests/test_ytdlp_extractor.py @@ -20,7 +20,7 @@ class TestProcessPoolConfiguration: assert kwargs == {"mp_context": context} get_context.assert_called_once_with("fork") - def test_uses_default_context_when_not_frozen_linux(self, monkeypatch): + def test_default_context_linux(self, monkeypatch): monkeypatch.setattr("app.features.ytdlp.extractor.sys.platform", "linux") monkeypatch.delattr("app.features.ytdlp.extractor.sys.frozen", raising=False) @@ -30,7 +30,7 @@ class TestProcessPoolConfiguration: assert _get_process_pool_kwargs() == {} get_context.assert_not_called() - def test_initializes_process_pool_with_context_kwargs(self, monkeypatch): + def test_init_pool_kwargs(self, monkeypatch): context = object() monkeypatch.setattr("app.features.ytdlp.extractor._get_process_pool_kwargs", lambda: {"mp_context": context}) diff --git a/app/features/ytdlp/tests/test_ytdlp_mini_filter.py b/app/features/ytdlp/tests/test_ytdlp_mini_filter.py index 307a9515..66b31824 100644 --- a/app/features/ytdlp/tests/test_ytdlp_mini_filter.py +++ b/app/features/ytdlp/tests/test_ytdlp_mini_filter.py @@ -195,7 +195,7 @@ class TestMiniFilter(unittest.TestCase): self._test("filesize>=1GiB", {"filesize": 2**30}, expected_result=True, test_name="filesize_1gib_positive") self._test("filesize>=1GiB", {"filesize": 1000000}, expected_result=False, test_name="filesize_1gib_negative") - def test_complex_duration_units_with_or_and_grouping(self): + def test_duration_or_grouping(self): """Test complex expressions with duration units, OR operations, and grouping. Skip yt-dlp due to known parsing bugs.""" # Test grouping with duration units expr = "(filesize>1MB & duration<10m) || uploader='BBC'" @@ -310,7 +310,7 @@ class TestMiniFilter(unittest.TestCase): self._test(expr, {"title": "a(b)c"}, expected_result=True, test_name="quoted_parentheses_match") self._test(expr, {"title": "abc"}, expected_result=False, test_name="quoted_parentheses_no_match") - def test_escaped_ampersand_inside_quoted_regex(self): + def test_quoted_regex_ampersand(self): expr = r"description~='(?i)\bcats \& dogs\b'" parser = MiniFilter(expr) diff --git a/app/features/ytdlp/tests/test_ytdlp_module.py b/app/features/ytdlp/tests/test_ytdlp_module.py index 66215795..a8c60e51 100644 --- a/app/features/ytdlp/tests/test_ytdlp_module.py +++ b/app/features/ytdlp/tests/test_ytdlp_module.py @@ -1,5 +1,8 @@ from unittest.mock import MagicMock, Mock, patch +import pytest + +from app.features.ytdlp.outtmpl import rewrite_outtmpl from app.features.ytdlp.patches import patch_windows_popen_wait from app.features.ytdlp.utils import _DATA from app.features.ytdlp.ytdlp import YTDLP, _ArchiveProxy, ytdlp_options @@ -20,7 +23,7 @@ class TestArchiveProxy: assert p2.add("") is False @patch("app.features.ytdlp.archiver.Archiver.get_instance") - def test_contains_and_add_delegate_to_archiver(self, mock_get_instance) -> None: + def test_delegates_to_archiver(self, mock_get_instance) -> None: arch = MagicMock() mock_get_instance.return_value = arch @@ -44,7 +47,7 @@ class TestArchiveProxy: class TestYtDlpOptions: - def test_options_structure_and_no_suppresshelp(self) -> None: + def test_options_shape(self) -> None: opts = ytdlp_options() assert isinstance(opts, list) @@ -91,7 +94,7 @@ class TestYTDLP: return ytdlp @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL.__init__") - def test_init_patches_download_archive_param(self, mock_super_init) -> None: + def test_init_archive_param(self, mock_super_init) -> None: """Test that __init__ removes download_archive before calling super, then restores it.""" mock_super_init.return_value = None @@ -118,7 +121,7 @@ class TestYTDLP: assert ytdlp.archive._file == "/tmp/archive.txt" @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL.__init__") - def test_init_handles_no_download_archive(self, mock_super_init) -> None: + def test_init_no_archive(self, mock_super_init) -> None: """Test __init__ works correctly when download_archive is not in params.""" mock_super_init.return_value = None @@ -136,7 +139,7 @@ class TestYTDLP: assert not ytdlp.archive @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL.__init__") - def test_init_handles_none_params(self, mock_super_init) -> None: + def test_init_none_params(self, mock_super_init) -> None: """Test __init__ handles None params gracefully.""" mock_super_init.return_value = None @@ -147,7 +150,7 @@ class TestYTDLP: assert not ytdlp.archive @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL.__init__") - def test_init_patches_windows_popen_wait_once(self, mock_super_init) -> None: + def test_init_patches_wait(self, mock_super_init) -> None: mock_super_init.return_value = None class FakePopen: @@ -160,7 +163,7 @@ class TestYTDLP: assert getattr(FakePopen, "_ytptube_wait_patched", False) is True - def test_windows_wait_patch_uses_polling_for_blocking_wait(self) -> None: + def test_wait_patch_polls(self) -> None: calls: list[float | None] = [] class FakePopen: @@ -183,7 +186,7 @@ class TestYTDLP: assert result == 0 assert calls == [0.1, 0.1, 0.1] - def test_windows_wait_patch_preserves_explicit_timeout(self) -> None: + def test_wait_patch_timeout(self) -> None: calls: list[float | None] = [] class FakePopen: @@ -202,7 +205,7 @@ class TestYTDLP: assert calls == [5] @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL._delete_downloaded_files") - def test_delete_downloaded_files_skips_when_interrupted(self, mock_super_delete) -> None: + def test_delete_interrupted(self, mock_super_delete) -> None: """Test _delete_downloaded_files skips cleanup when _interrupted is True.""" with patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL.__init__", return_value=None): ytdlp = YTDLP(params={}) @@ -221,7 +224,7 @@ class TestYTDLP: assert result is None @patch("app.features.ytdlp.ytdlp.yt_dlp.YoutubeDL._delete_downloaded_files") - def test_delete_downloaded_files_calls_super_when_not_interrupted(self, mock_super_delete) -> None: + def test_delete_calls_super(self, mock_super_delete) -> None: """Test _delete_downloaded_files calls super when not interrupted.""" mock_super_delete.return_value = "cleanup_result" @@ -236,7 +239,7 @@ class TestYTDLP: # Should return super's result assert result == "cleanup_result" - def test_record_download_archive_does_nothing_without_download_archive_param(self) -> None: + def test_record_archive_missing(self) -> None: """Test record_download_archive returns early when download_archive is not set.""" ytdlp = self._create_ytdlp(params={}) ytdlp.archive = Mock() @@ -246,7 +249,7 @@ class TestYTDLP: # Should not interact with archive ytdlp.archive.add.assert_not_called() - def test_record_download_archive_adds_archive_id(self) -> None: + def test_record_archive_adds_id(self) -> None: """Test record_download_archive adds the archive ID.""" ytdlp = self._create_ytdlp(params={"download_archive": "/tmp/archive.txt"}) ytdlp.write_debug = Mock() @@ -264,7 +267,7 @@ class TestYTDLP: ytdlp.archive.add.assert_called_once_with("youtube test123") ytdlp.write_debug.assert_called_with("Adding to archive: youtube test123") - def test_record_download_archive_handles_old_archive_ids(self) -> None: + def test_record_archive_old_ids(self) -> None: """Test record_download_archive adds _old_archive_ids when present.""" ytdlp = self._create_ytdlp(params={"download_archive": "/tmp/archive.txt"}) ytdlp.write_debug = Mock() @@ -291,7 +294,7 @@ class TestYTDLP: # Should not add duplicate (youtube new123 appears only once) assert calls.count("youtube new123") == 1 - def test_record_download_archive_skips_empty_old_archive_ids(self) -> None: + def test_record_archive_empty_old_ids(self) -> None: """Test record_download_archive handles empty or invalid _old_archive_ids.""" ytdlp = self._create_ytdlp(params={"download_archive": "/tmp/archive.txt"}) ytdlp.write_debug = Mock() @@ -317,7 +320,7 @@ class TestYTDLP: ytdlp.record_download_archive(info_dict) assert ytdlp.archive.add.call_count == 1 # Only main ID - def test_record_download_archive_returns_early_on_empty_archive_id(self) -> None: + def test_record_archive_empty_id(self) -> None: """Test record_download_archive returns early when _make_archive_id returns empty.""" ytdlp = self._create_ytdlp(params={"download_archive": "/tmp/archive.txt"}) ytdlp.archive = Mock() @@ -327,3 +330,116 @@ class TestYTDLP: # Should not add anything ytdlp.archive.add.assert_not_called() + + def test_outtmpl_callable(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + result = ytdlp.evaluate_outtmpl("%(ytp_random:8)s", {"title": "x"}) + + assert len(result) == 8 + assert result.isalnum() + + def test_outtmpl_reuses_value(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + result = ytdlp.evaluate_outtmpl("%(ytp_random:8)s/%(ytp_random:8)s", {"title": "x"}) + first, second = result.split("/") + + assert first == second + + def test_outtmpl_new_value(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + first = ytdlp.evaluate_outtmpl("%(ytp_random:8)s", {"title": "x"}) + second = ytdlp.evaluate_outtmpl("%(ytp_random:8)s", {"title": "y"}) + + assert first != second + + def test_prepare_filename_sidecars(self) -> None: + ytdlp = YTDLP( + params={ + "outtmpl": { + "default": "%(ytp_random:6)s.%(ext)s", + "thumbnail": "%(ytp_random:6)s.%(ext)s", + "subtitle": "%(ytp_random:6)s.%(ext)s", + "infojson": "%(ytp_random:6)s.%(ext)s", + } + } + ) + + info = {"id": "abc123", "title": "Example", "ext": "mp4"} + + default_name = ytdlp.prepare_filename(info) + thumbnail_name = ytdlp.prepare_filename(info, "thumbnail") + subtitle_name = ytdlp.prepare_filename(info, "subtitle") + infojson_name = ytdlp.prepare_filename(info, "infojson") + + default_base = default_name.rsplit(".", 1)[0] + thumbnail_base = thumbnail_name.rsplit(".", 1)[0] + subtitle_base = subtitle_name.rsplit(".", 1)[0] + infojson_base = infojson_name.removesuffix(".info.json") + + assert default_base == thumbnail_base + assert default_base == subtitle_base + assert default_base == infojson_base + + def test_prepare_filename_resets(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(ytp_random:8)s.%(ext)s"}}) + + first = ytdlp.prepare_filename({"id": "one", "title": "One", "ext": "mp4"}) + second = ytdlp.prepare_filename({"id": "two", "title": "Two", "ext": "mp4"}) + + assert first != second + + @pytest.mark.parametrize( + ("template", "expected"), + [ + ("%(ytp_random:8|fallback)s", 8), + ("%(ytp_random:8&{} - |)s", 11), + ("%(ytp_random:8)S", 8), + ], + ) + def test_outtmpl_suffix(self, template: str, expected: int) -> None: + ytdlp = YTDLP( + params={ + "outtmpl": {"default": "%(title)s"}, + "restrictfilenames": True, + }, + ) + + result = ytdlp.evaluate_outtmpl(template, {"title": "x"}) + + assert len(result) == expected + + def test_outtmpl_modes(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + digits = ytdlp.evaluate_outtmpl("%(ytp_random:6:d)s", {"title": "x"}) + letters = ytdlp.evaluate_outtmpl("%(ytp_random:6:s)s", {"title": "x"}) + + assert digits.isdigit() + assert letters.isalpha() + + def test_outtmpl_unknown_callable(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + with pytest.raises(ValueError, match="Unsupported YTPTube output template callable"): + ytdlp.prepare_outtmpl("%(ytp_unknown:8)s", {"title": "x"}) + + def test_outtmpl_invalid_length(self) -> None: + ytdlp = YTDLP(params={"outtmpl": {"default": "%(title)s"}}) + + with pytest.raises(ValueError, match="ytp_random length must be an integer"): + ytdlp.prepare_outtmpl("%(ytp_random:nope)s", {"title": "x"}) + + +class TestOuttmpl: + def test_rewrite_outtmpl_cache(self) -> None: + cache: dict[str, object] = {} + template = "%(ytp_random:8)s/%(ytp_random:8)s.%(ext)s" + + rewritten, info = rewrite_outtmpl(template, {"ext": "mp4"}, cache=cache) + + assert rewritten == "%(__ytptube_outtmpl_0)s/%(__ytptube_outtmpl_0)s.%(ext)s" + assert len(cache) == 1 + assert info["__ytptube_outtmpl_0"] == next(iter(cache.values())) diff --git a/app/features/ytdlp/tests/test_ytdlp_opts.py b/app/features/ytdlp/tests/test_ytdlp_opts.py index bb84391b..3bc15451 100644 --- a/app/features/ytdlp/tests/test_ytdlp_opts.py +++ b/app/features/ytdlp/tests/test_ytdlp_opts.py @@ -25,7 +25,7 @@ class TestYTDLPOpts: assert opts._item_cli == [] assert opts._preset_cli == "" - def test_get_instance_returns_reset_instance(self): + def test_get_instance(self): """Test that get_instance returns a reset YTDLPOpts instance.""" with patch("app.features.ytdlp.ytdlp_opts.Config"): opts = YTDLPOpts.get_instance() @@ -51,7 +51,7 @@ class TestYTDLPOpts: assert "--format best" in opts._item_cli mock_converter.assert_called_once_with(args="--format best", level=False) - def test_add_cli_with_invalid_args_raises_error(self): + def test_add_cli_invalid(self): """Test that invalid CLI arguments raise ValueError.""" with ( patch("app.features.ytdlp.ytdlp_opts.Config"), @@ -63,7 +63,7 @@ class TestYTDLPOpts: with pytest.raises(ValueError, match="Invalid command options for yt-dlp were given"): opts.add_cli("--invalid-arg", from_user=True) - def test_add_cli_with_empty_args_returns_self(self): + def test_add_cli_empty(self): """Test that empty or invalid args return self without processing.""" with patch("app.features.ytdlp.ytdlp_opts.Config"): opts = YTDLPOpts() @@ -95,7 +95,7 @@ class TestYTDLPOpts: assert opts._item_opts["format"] == "best" assert opts._item_opts["quality"] == "720p" - def test_add_with_user_config_filters_bad_options(self): + def test_add_filters_bad_options(self): """Test that user config filters out dangerous options.""" with patch("app.features.ytdlp.ytdlp_opts.Config"): opts = YTDLPOpts() @@ -211,7 +211,7 @@ class TestYTDLPOpts: assert opts._preset_opts["cookiefile"] == expected_path mock_preset.get_cookies_file.assert_called_once_with(config=mock_config_instance) - def test_preset_with_invalid_cli_raises_error(self): + def test_preset_invalid_cli(self): """Test that preset with invalid CLI raises ValueError.""" with ( patch("app.features.ytdlp.ytdlp_opts.Config"), @@ -289,7 +289,7 @@ class TestYTDLPOpts: expected_cli = "--quality 720p\n--format best" mock_converter.assert_called_once_with(args=expected_cli, level=True) - def test_get_all_handles_format_special_cases(self): + def test_get_all_format_cases(self): """Test get_all handles special format values correctly.""" with ( patch("app.features.ytdlp.ytdlp_opts.Config") as mock_config, @@ -325,7 +325,7 @@ class TestYTDLPOpts: result = opts.get_all(keep=True) assert result["format"] == "best" - def test_get_all_with_invalid_cli_raises_error(self): + def test_get_all_invalid_cli(self): """Test get_all raises error for invalid CLI arguments.""" with ( patch("app.features.ytdlp.ytdlp_opts.Config") as mock_config, @@ -578,7 +578,7 @@ class TestARGSMerger: assert "--output" in merger.args assert "--socket-timeout" in merger.args - def test_add_filters_complex_commented_extractor_args(self): + def test_add_complex_comments(self): """Test filtering of complex real-world commented extractor-args.""" from app.features.ytdlp.ytdlp_opts import ARGSMerger @@ -631,7 +631,7 @@ class TestARGSMerger: assert "--format" in merger.args assert "bestvideo[height<=1080]+bestaudio/best" in merger.args - def test_add_empty_string_returns_self(self): + def test_add_empty(self): """Test that adding empty string returns self without modifying args.""" from app.features.ytdlp.ytdlp_opts import ARGSMerger @@ -641,7 +641,7 @@ class TestARGSMerger: assert result is merger assert merger.args == [] - def test_add_short_string_returns_self(self): + def test_add_short(self): """Test that adding short string (len < 2) returns self without modifying args.""" from app.features.ytdlp.ytdlp_opts import ARGSMerger @@ -651,7 +651,7 @@ class TestARGSMerger: assert result is merger assert merger.args == [] - def test_add_non_string_returns_self(self): + def test_add_non_string(self): """Test that adding non-string returns self without modifying args.""" from app.features.ytdlp.ytdlp_opts import ARGSMerger @@ -769,7 +769,7 @@ class TestYTDLPCli: assert cli.item is item assert cli._config is mock_config_instance - def test_constructor_with_invalid_type_raises_error(self): + def test_constructor_invalid_item(self): """Test YTDLPCli constructor raises error with non-Item type.""" from app.features.ytdlp.ytdlp_opts import YTDLPCli diff --git a/app/features/ytdlp/tests/test_ytdlp_utils.py b/app/features/ytdlp/tests/test_ytdlp_utils.py index 863fa7ee..cf84e5fc 100644 --- a/app/features/ytdlp/tests/test_ytdlp_utils.py +++ b/app/features/ytdlp/tests/test_ytdlp_utils.py @@ -48,14 +48,14 @@ class TestLogWrapper: with pytest.raises(TypeError, match=r"Target must be a logging\.Logger instance or a callable"): lw.add_target(123) # type: ignore[arg-type] - def test_add_target_name_inference_and_custom(self) -> None: + def test_add_target_names(self) -> None: lw = LogWrapper() logger, _ = make_logger("one") # Name inferred from logger lw.add_target(logger) assert lw.targets[-1].name == "one" - assert lw.has_targets() is True + assert lw.has_targets() # Name inferred from callable def sink(level: int, msg: str, *args: Any, **kwargs: Any) -> None: # noqa: ARG001 @@ -129,7 +129,7 @@ class TestExtractYtdlpLogs: logs = ["INFO: Downloading", "ERROR: Failed", "WARNING: Deprecated"] filters = [re.compile(r"ERROR")] result = extract_ytdlp_logs(logs, filters) - assert len(result) >= 0 # Should filter based on patterns + assert result == ["ERROR: Failed"] def test_extract_ytdlp_logs_empty(self): """Test with empty logs.""" @@ -146,8 +146,8 @@ class TestYtdlpReject: yt_params = {} passed, message = ytdlp_reject(entry, yt_params) - assert isinstance(passed, bool) - assert isinstance(message, str) + assert passed is True + assert message == "" def test_ytdlp_reject_with_filters(self): """Test rejection with filters.""" @@ -158,8 +158,9 @@ class TestYtdlpReject: yt_params["daterange"].__contains__ = MagicMock(return_value=False) passed, message = ytdlp_reject(entry, yt_params) - assert isinstance(passed, bool) - assert isinstance(message, str) + assert passed is False + assert "20230101" in message + assert "not in range" in message class TestParseOuttmpl: @@ -256,7 +257,7 @@ class TestParseOuttmpl: assert result == "Test Channel/Best Videos/005 - Amazing Content [abc123xyz].mp4" - def test_parse_outtmpl_with_special_characters(self): + def test_parse_outtmpl_special_chars(self): """Test template parsing handles special characters in values.""" template = "%(title)s.%(ext)s" @@ -284,7 +285,7 @@ class TestParseOuttmpl: assert result == "My Playlist/Video Title.webm" - def test_parse_outtmpl_with_restrict_filename(self): + def test_parse_outtmpl_restrict(self): """Test template parsing with restrict_filename parameter.""" template = "%(uploader)s/%(title)s.%(ext)s" @@ -302,19 +303,19 @@ class TestParseOuttmpl: class TestGetThumbnail: - def test_returns_none_for_empty_list(self): + def test_empty_list(self): """Test that None is returned for an empty thumbnail list.""" assert get_thumbnail([]) is None - def test_returns_none_for_non_list(self): + def test_non_list(self): """Test that None is returned for non-list input.""" assert get_thumbnail(None) is None assert get_thumbnail("not a list") is None assert get_thumbnail({"not": "list"}) is None - def test_returns_highest_preference_thumbnail(self): + def test_thumbnail_preference(self): """Test that the thumbnail with highest preference is returned.""" thumbnails = [ @@ -326,7 +327,7 @@ class TestGetThumbnail: result = get_thumbnail(thumbnails) assert result == {"url": "high.jpg", "preference": 10, "width": 200, "height": 200} - def test_returns_highest_width_when_preference_equal(self): + def test_thumbnail_width(self): """Test that the thumbnail with highest width is returned when preference is equal.""" thumbnails = [ @@ -338,7 +339,7 @@ class TestGetThumbnail: result = get_thumbnail(thumbnails) assert result == {"url": "large.jpg", "preference": 1, "width": 200, "height": 200} - def test_handles_missing_attributes(self): + def test_missing_attrs(self): """Test that thumbnails with missing attributes are handled correctly.""" thumbnails = [ @@ -349,7 +350,7 @@ class TestGetThumbnail: result = get_thumbnail(thumbnails) assert result["url"] == "with_pref.jpg" - def test_returns_first_when_all_equal(self): + def test_all_equal(self): """Test that any thumbnail is returned when all attributes are equal.""" thumbnails = [ @@ -357,17 +358,15 @@ class TestGetThumbnail: {"url": "second.jpg"}, ] - result = get_thumbnail(thumbnails) - assert result is not None - assert result["url"] in ["first.jpg", "second.jpg"] + assert get_thumbnail(thumbnails) == {"url": "second.jpg"} class TestGetExtras: - def test_returns_empty_dict_for_none(self): + def test_none(self): """Test that empty dict is returned for None input.""" assert get_extras(None) == {} - def test_returns_empty_dict_for_non_dict(self): + def test_non_dict(self): """Test that empty dict is returned for non-dict input.""" assert get_extras("not a dict") == {} assert get_extras([]) == {} @@ -409,7 +408,7 @@ class TestGetExtras: assert result["playlist_uploader"] == "Playlist Owner" assert result["playlist_uploader_id"] == "owner123" - def test_handles_release_timestamp(self): + def test_release_timestamp(self): """Test handling of release_timestamp for upcoming content.""" entry = { @@ -421,7 +420,7 @@ class TestGetExtras: assert "release_in" in result assert result["release_in"] == "Fri, 13 Feb 2009 23:31:30 GMT" - def test_handles_upcoming_live_stream(self): + def test_upcoming_live(self): """Test handling of upcoming live stream.""" entry = { @@ -434,7 +433,7 @@ class TestGetExtras: assert result["is_live"] == 1234567890 assert "release_in" in result - def test_handles_premiere_flag(self): + def test_premiere_flag(self): """Test handling of is_premiere flag.""" entry = { @@ -481,7 +480,7 @@ class TestGetStaticYtdlp: _DATA.YTDLP_INFO_CLS = None - def test_get_static_ytdlp_returns_instance(self): + def test_instance(self): """Test that get_static_ytdlp returns a YTDLP instance.""" from app.features.ytdlp.ytdlp import YTDLP @@ -491,7 +490,7 @@ class TestGetStaticYtdlp: assert instance is not None assert isinstance(instance, YTDLP) - def test_get_static_ytdlp_returns_same_instance(self): + def test_get_static_ytdlp_same(self): """Test that get_static_ytdlp returns the same cached instance.""" instance1 = get_ytdlp() @@ -508,7 +507,7 @@ class TestGetStaticYtdlp: assert instance1 is not instance2 assert instance2 is not None - def test_get_static_ytdlp_has_correct_params(self): + def test_get_static_ytdlp_params(self): """Test that get_static_ytdlp initializes with correct parameters.""" instance = get_ytdlp() @@ -560,22 +559,22 @@ class TestArchiveFunctions: def test_archive_add_and_read(self): """Test adding and reading archive entries.""" - ids = ["id1", "id2", "id3"] + ids = ["youtube id1", "youtube id2", "youtube id3"] - # Add entries - just test it returns a boolean result = archive_add(self.archive_file, ids) - assert isinstance(result, bool) + assert result is True - # Read entries - just test it returns a list read_ids = archive_read(self.archive_file) - assert isinstance(read_ids, list) + assert set(read_ids) == set(ids) def test_archive_delete(self): """Test deleting archive entries.""" - # Delete some entries - just test it returns a boolean - delete_ids = ["id2"] + archive_add(self.archive_file, ["youtube id1", "youtube id2", "youtube id3"]) + + delete_ids = ["youtube id2"] result = archive_delete(self.archive_file, delete_ids) - assert isinstance(result, bool) + assert result is True + assert set(archive_read(self.archive_file)) == {"youtube id1", "youtube id3"} def test_archive_read_nonexistent(self): """Test reading from non-existent archive.""" diff --git a/app/features/ytdlp/ytdlp.py b/app/features/ytdlp/ytdlp.py index cccba7e5..9f4394d9 100644 --- a/app/features/ytdlp/ytdlp.py +++ b/app/features/ytdlp/ytdlp.py @@ -5,6 +5,7 @@ from typing import Any import yt_dlp from yt_dlp.utils import make_archive_id +from app.features.ytdlp.outtmpl import rewrite_outtmpl from app.features.ytdlp.patches import apply_ytdlp_patches from app.library.cf_solver_handler import set_cf_handler @@ -76,6 +77,8 @@ class YTDLP(yt_dlp.YoutubeDL): except Exception: pass + self._ytptube_outtmpl_info: dict[str, Any] | None = None + self._ytptube_outtmpl_cache: dict[str, Any] = {} self.archive = _ArchiveProxy(orig_file) if not YTDLP._registered: try: @@ -94,6 +97,25 @@ class YTDLP(yt_dlp.YoutubeDL): return super()._delete_downloaded_files(*args, **kwargs) + def _reset_outtmpl_cache(self) -> None: + self._ytptube_outtmpl_info = None + self._ytptube_outtmpl_cache = {} + + def prepare_outtmpl(self, outtmpl, info_dict, sanitize=False): + if self._ytptube_outtmpl_info is not info_dict: + self._ytptube_outtmpl_info = info_dict + self._ytptube_outtmpl_cache = {} + + outtmpl, enriched = rewrite_outtmpl(outtmpl, info_dict, cache=self._ytptube_outtmpl_cache) + + return super().prepare_outtmpl(outtmpl, enriched, sanitize=sanitize) + + def process_info(self, info_dict): + try: + return super().process_info(info_dict) + finally: + self._reset_outtmpl_cache() + def record_download_archive(self, info_dict) -> None: if not self.params.get("download_archive"): return diff --git a/app/library/HttpAPI.py b/app/library/HttpAPI.py index 95bf9c8a..e956b5c0 100644 --- a/app/library/HttpAPI.py +++ b/app/library/HttpAPI.py @@ -256,7 +256,7 @@ class HttpAPI: response: Response = await handler(request) - contentType: str = response.headers.get("content-type", "") + contentType: str = str(response.headers.get("content-type", "")) if contentType.startswith("text/html") and getattr(response, "_path", None): rewrite_path: str = base_path.rstrip("/") async with await anyio.open_file(response._path, "rb") as f: diff --git a/app/library/TerminalSessionManager.py b/app/library/TerminalSessionManager.py index 423dd131..07edb9a1 100644 --- a/app/library/TerminalSessionManager.py +++ b/app/library/TerminalSessionManager.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, Any from aiohttp import web from app.library.config import Config +from app.library.Scheduler import Scheduler from app.library.Services import Services from app.library.Singleton import Singleton @@ -31,8 +32,10 @@ ACTIVE_FILE_NAME = "active.json" METADATA_FILE_NAME = "metadata.json" TRANSCRIPT_FILE_NAME = "transcript.jsonl" DEFAULT_DRAIN_TTL = 30.0 +COMPLETED_SESSION_RETENTION = 86400.0 DEFAULT_KEEPALIVE_INTERVAL = 15.0 DEFAULT_SHUTDOWN_TIMEOUT = 5.0 +CLEANUP_SCHEDULE = "5 */1 * * *" class TerminalSessionConflictError(RuntimeError): @@ -55,8 +58,10 @@ class TerminalSessionManager(metaclass=Singleton): self._lock = asyncio.Lock() self._active: ActiveTerminalSession | None = None self._drain_ttl: float = DEFAULT_DRAIN_TTL + self._completed_retention: float = COMPLETED_SESSION_RETENTION self._keepalive_interval: float = DEFAULT_KEEPALIVE_INTERVAL self._shutdown_timeout: float = DEFAULT_SHUTDOWN_TIMEOUT + self._cleanup_job_id: str | None = None @staticmethod def get_instance() -> TerminalSessionManager: @@ -67,11 +72,20 @@ class TerminalSessionManager(metaclass=Singleton): Services.get_instance().add("terminal_manager", self) app.on_startup.append(self.on_startup) app.on_shutdown.append(self.on_shutdown) + self._cleanup_job_id = Scheduler.get_instance().add( + timer=CLEANUP_SCHEDULE, + func=self.cleanup, + id=f"{__class__.__name__}.{__class__.cleanup.__name__}", + ) async def on_startup(self, _: web.Application) -> None: await self.initialize() async def on_shutdown(self, _: web.Application) -> None: + if self._cleanup_job_id is not None: + Scheduler.get_instance().remove(self._cleanup_job_id) + self._cleanup_job_id = None + session_id: str | None = None task: asyncio.Task[None] | None = None @@ -121,12 +135,17 @@ class TerminalSessionManager(metaclass=Singleton): async def cleanup(self) -> None: async with self._lock: - self._cleanup_expired_sessions(time.time()) + LOG.debug("Running scheduled terminal session cleanup.") + removed = self._cleanup_expired_sessions(time.time()) + LOG.debug("Scheduled terminal session cleanup finished. Removed %d expired sessions.", removed) + + async def list_sessions(self) -> list[dict[str, Any]]: + async with self._lock: + return self._list_sessions_locked() async def create_session(self, command: str) -> dict[str, Any]: async with self._lock: now = time.time() - self._cleanup_expired_sessions(now) active_session = self._get_active_session_locked() if active_session is not None: @@ -160,22 +179,20 @@ class TerminalSessionManager(metaclass=Singleton): async def get_active_session(self) -> dict[str, Any] | None: async with self._lock: - self._cleanup_expired_sessions(time.time()) metadata = self._get_active_session_locked() return None if metadata is None else dict(metadata) async def get_session(self, session_id: str) -> dict[str, Any] | None: async with self._lock: - self._cleanup_expired_sessions(time.time()) metadata = self._load_metadata(session_id) - return None if metadata is None else dict(metadata) + if metadata is None or self._is_expired_metadata(metadata, time.time()): + return None + return dict(metadata) async def cancel_session(self, session_id: str) -> dict[str, Any]: async with self._lock: - self._cleanup_expired_sessions(time.time()) - metadata = self._load_metadata(session_id) - if metadata is None: + if metadata is None or self._is_expired_metadata(metadata, time.time()): msg = f"Unknown terminal session '{session_id}'." raise FileNotFoundError(msg) @@ -218,6 +235,9 @@ class TerminalSessionManager(metaclass=Singleton): async with self._lock: metadata = self._load_metadata(session_id) if metadata is not None: + if self._is_expired_metadata(metadata, time.time()): + return response + replay_until = int(metadata.get("last_sequence", last_sent)) runtime = self._active @@ -443,7 +463,7 @@ class TerminalSessionManager(metaclass=Singleton): metadata["status"] = status metadata["exit_code"] = exit_code metadata["finished_at"] = now - metadata["expires_at"] = now + self._drain_ttl + metadata["expires_at"] = now + self._completed_retention self._write_json(self._metadata_path(session_id), metadata) if self._load_active_marker() == session_id: @@ -468,6 +488,31 @@ class TerminalSessionManager(metaclass=Singleton): return metadata + def _list_sessions_locked(self) -> list[dict[str, Any]]: + self._ensure_root() + items: list[dict[str, Any]] = [] + + for path in self.root_path.iterdir(): + if path.name == ACTIVE_FILE_NAME or path.is_dir() is False: + continue + + metadata = self._load_metadata(path.name) + if metadata is None: + continue + + if self._is_expired_metadata(metadata, time.time()): + continue + + item = dict(metadata) + item["available_until"] = self._available_until(metadata) + items.append(item) + + items.sort( + key=lambda item: float(item.get("finished_at") or item.get("started_at") or item.get("created_at") or 0), + reverse=True, + ) + return items + def _recover_orphaned_active_session(self, now: float) -> None: session_id = self._load_active_marker() if session_id is None: @@ -480,14 +525,15 @@ class TerminalSessionManager(metaclass=Singleton): metadata["status"] = "interrupted" metadata["finished_at"] = now - metadata["expires_at"] = now + self._drain_ttl + metadata["expires_at"] = now + self._completed_retention metadata["exit_code"] = -1 if metadata.get("exit_code") is None else metadata["exit_code"] self._write_json(self._metadata_path(session_id), metadata) self._clear_active_marker() - def _cleanup_expired_sessions(self, now: float) -> None: + def _cleanup_expired_sessions(self, now: float) -> int: self._ensure_root() active_session_id = self._load_active_marker() + removed = 0 for path in self.root_path.iterdir(): if path.name == ACTIVE_FILE_NAME or path.is_dir() is False: @@ -496,10 +542,10 @@ class TerminalSessionManager(metaclass=Singleton): metadata = self._load_metadata(path.name) if metadata is None: shutil.rmtree(path, ignore_errors=True) + removed += 1 continue - expires_at = metadata.get("expires_at") - if expires_at is None or float(expires_at) > now: + if not self._is_expired_metadata(metadata, now): continue if active_session_id == path.name: @@ -508,6 +554,10 @@ class TerminalSessionManager(metaclass=Singleton): shutil.rmtree(path, ignore_errors=True) + removed += 1 + + return removed + def _parse_since(self, request: Request) -> int: values: list[int] = [] candidates = [request.query.get("since"), request.headers.get("Last-Event-ID")] @@ -537,7 +587,7 @@ class TerminalSessionManager(metaclass=Singleton): now = time.time() metadata["status"] = "interrupted" metadata["finished_at"] = now - metadata["expires_at"] = now + self._drain_ttl + metadata["expires_at"] = now + self._completed_retention metadata["exit_code"] = -1 if metadata.get("exit_code") is None else metadata["exit_code"] self._write_json(self._metadata_path(session_id), metadata) @@ -618,6 +668,20 @@ class TerminalSessionManager(metaclass=Singleton): return events + def _available_until(self, metadata: dict[str, Any]) -> float | None: + expires_at = metadata.get("expires_at") + if expires_at is None: + return None + + return float(expires_at) + + def _is_expired_metadata(self, metadata: dict[str, Any], now: float) -> bool: + expires_at = self._available_until(metadata) + if expires_at is None: + return False + + return expires_at <= now + def _build_env(self) -> dict[str, str]: env_vars = os.environ.copy() env_vars.update( diff --git a/app/routes/api/history.py b/app/routes/api/history.py index 1da5baa7..a063cfc5 100644 --- a/app/routes/api/history.py +++ b/app/routes/api/history.py @@ -1,5 +1,6 @@ import asyncio import logging +from pathlib import Path from typing import TYPE_CHECKING, Any from aiohttp import web @@ -10,10 +11,12 @@ from app.features.presets.service import Presets from app.library.config import Config from app.library.DataStore import StoreType from app.library.downloads import Download, DownloadQueue +from app.library.downloads.utils import safe_relative_path from app.library.encoder import Encoder from app.library.Events import EventBus, Events from app.library.ItemDTO import Item from app.library.router import route +from app.library.Utils import calc_download_path, get_file_sidecar, rename_file if TYPE_CHECKING: from library.downloads import Download @@ -287,9 +290,9 @@ async def item_view(request: Request, queue: DownloadQueue, encoder: Encoder) -> } if "finished" == item.info.status and (filename := item.info.get_file()): - from app.features.streaming.library.ffprobe import ffprobe - try: + from app.features.streaming.library.ffprobe import ffprobe + info["ffprobe"] = await ffprobe(filename) except Exception: pass @@ -302,6 +305,70 @@ async def item_view(request: Request, queue: DownloadQueue, encoder: Encoder) -> return web.json_response(data=info, status=web.HTTPOk.status_code, dumps=encoder.encode) +@route("POST", r"api/history/{id}/rename", "history.item.rename") +async def item_rename( + request: Request, + queue: DownloadQueue, + encoder: Encoder, + notify: EventBus, + config: Config, +) -> Response: + if not (id := request.match_info.get("id")): + return web.json_response(data={"error": "id is required."}, status=web.HTTPBadRequest.status_code) + + item: Download | None = await queue.done.get_by_id(id) + if not item: + return web.json_response(data={"error": f"item '{id}' not found."}, status=web.HTTPNotFound.status_code) + + try: + post: dict = await request.json() + if not post: + return web.json_response(data={"error": "no data provided."}, status=web.HTTPBadRequest.status_code) + except Exception: + return web.json_response(data={"error": "invalid JSON body."}, status=web.HTTPBadRequest.status_code) + + new_name: str = str(post.get("new_name") or "").strip() + if not new_name: + return web.json_response(data={"error": "new_name is required."}, status=web.HTTPBadRequest.status_code) + + filepath: Path | None = item.info.get_file(download_path=Path(config.download_path)) + if not filepath: + return web.json_response(data={"error": "item has no downloaded file."}, status=web.HTTPBadRequest.status_code) + + try: + renamed, sidecar_renamed = rename_file(filepath, new_name) + except ValueError as e: + return web.json_response(data={"error": str(e)}, status=web.HTTPConflict.status_code) + except OSError as e: + LOG.exception(e) + return web.json_response(data={"error": str(e)}, status=web.HTTPInternalServerError.status_code) + + item_dir: Path = ( + Path(item.info.download_dir) + if item.info.download_dir + else Path(calc_download_path(base_path=config.download_path, folder=item.info.folder, create_path=False)) + ) + item.info.filename = safe_relative_path(renamed, item_dir, Path(config.download_path)) + + try: + item.info.sidecar = get_file_sidecar(renamed) + except Exception: + item.info.sidecar = {} + + await queue.done.put(item, no_notify=True) + notify.emit(Events.ITEM_UPDATED, data=item.info) + + sidecar_count: int = len(sidecar_renamed) + sidecar_msg: str = f" and {sidecar_count} sidecar file/s" if sidecar_count > 0 else "" + LOG.info(f"Renamed file '{filepath}' to '{renamed}'{sidecar_msg}") + + return web.json_response( + data=item.info, + status=web.HTTPOk.status_code, + dumps=encoder.encode, + ) + + @route("POST", "api/history/{id}", "item_update") async def item_update(request: Request, queue: DownloadQueue, encoder: Encoder, notify: EventBus) -> Response: """ diff --git a/app/routes/api/system.py b/app/routes/api/system.py index 5d157156..f45f5f3f 100644 --- a/app/routes/api/system.py +++ b/app/routes/api/system.py @@ -274,6 +274,20 @@ async def create_terminal_session( return web.json_response(data=metadata, status=web.HTTPOk.status_code, dumps=encoder.encode) +@route("GET", "api/system/terminal", "system.terminal.list") +async def list_terminal_sessions( + config: Config, encoder: Encoder, terminal_manager: TerminalSessionManager +) -> Response: + if not config.console_enabled: + return web.json_response( + {"error": "Console feature is disabled."}, + status=web.HTTPForbidden.status_code, + ) + + items = await terminal_manager.list_sessions() + return web.json_response(data={"items": items}, status=web.HTTPOk.status_code, dumps=encoder.encode) + + @route("GET", "api/system/terminal/active", "system.terminal.active") async def get_active_terminal_session( config: Config, encoder: Encoder, terminal_manager: TerminalSessionManager diff --git a/app/tests/helpers.py b/app/tests/helpers.py index b01f8c21..d1a48027 100644 --- a/app/tests/helpers.py +++ b/app/tests/helpers.py @@ -11,7 +11,7 @@ from uuid import uuid4 if TYPE_CHECKING: from collections.abc import Iterator -_TEST_RUN_ROOT = Path(gettempdir()) / "ytptube-tests" / uuid4().hex +_TEST_RUN_ROOT = Path(gettempdir()) / "tests-ytptube" / uuid4().hex _TEST_SYSTEM_TEMP_ROOT = _TEST_RUN_ROOT / "tmp" diff --git a/app/tests/test_cache.py b/app/tests/test_cache.py index c03de8e8..bb7524c2 100644 --- a/app/tests/test_cache.py +++ b/app/tests/test_cache.py @@ -41,7 +41,7 @@ class TestCache: time.sleep(0.2) assert self.cache.get("temp_key") is None - def test_set_without_ttl(self): + def test_set_no_ttl(self): """Test setting values without TTL (permanent).""" self.cache.set("permanent_key", "permanent_value") assert self.cache.get("permanent_key") == "permanent_value" diff --git a/app/tests/test_condition_ignore.py b/app/tests/test_condition_ignore.py index f107b263..3991542f 100644 --- a/app/tests/test_condition_ignore.py +++ b/app/tests/test_condition_ignore.py @@ -18,7 +18,7 @@ def reset_conditions_singleton(): class TestConditionIgnoreMatching: @pytest.mark.asyncio - async def test_match_skips_condition_by_name_and_returns_next_match(self) -> None: + async def test_match_skip_name(self) -> None: first = SimpleNamespace(id=1, name="Primary", enabled=True, filter="duration > 0", priority=20) second = SimpleNamespace(id=2, name="Fallback", enabled=True, filter="duration > 0", priority=10) @@ -30,7 +30,7 @@ class TestConditionIgnoreMatching: assert matched is second @pytest.mark.asyncio - async def test_match_skips_condition_by_stringified_id_and_returns_next_match(self) -> None: + async def test_match_skip_id(self) -> None: first = SimpleNamespace(id=123, name="Primary", enabled=True, filter="duration > 0", priority=20) second = SimpleNamespace(id=124, name="Fallback", enabled=True, filter="duration > 0", priority=10) @@ -42,7 +42,7 @@ class TestConditionIgnoreMatching: assert matched is second @pytest.mark.asyncio - async def test_match_coerces_numeric_ignore_values_to_strings(self) -> None: + async def test_match_coerce_ids(self) -> None: first = SimpleNamespace(id=123, name="Primary", enabled=True, filter="duration > 0", priority=20) second = SimpleNamespace(id=124, name="Fallback", enabled=True, filter="duration > 0", priority=10) @@ -54,7 +54,7 @@ class TestConditionIgnoreMatching: assert matched is second @pytest.mark.asyncio - async def test_match_returns_none_when_ignore_all_wildcard_is_present(self) -> None: + async def test_match_wildcard(self) -> None: first = SimpleNamespace(id=1, name="Primary", enabled=True, filter="duration > 0", priority=20) service = Conditions.get_instance() @@ -67,7 +67,7 @@ class TestConditionIgnoreMatching: class TestConditionIgnorePropagation: @pytest.mark.asyncio - async def test_item_adder_passes_ignore_conditions_to_matcher(self) -> None: + async def test_add_passes_ignore(self) -> None: queue = Mock() queue.config = Mock(temp_path="/tmp", ignore_archived_items=False, ytdlp_debug=False) queue._notify = Mock() @@ -102,7 +102,7 @@ class TestConditionIgnorePropagation: assert result == {"status": "ok"} @pytest.mark.asyncio - async def test_item_adder_coerces_numeric_ignore_conditions_to_strings(self) -> None: + async def test_add_coerces_ignore(self) -> None: queue = Mock() queue.config = Mock(temp_path="/tmp", ignore_archived_items=False, ytdlp_debug=False) queue._notify = Mock() @@ -133,7 +133,7 @@ class TestConditionIgnorePropagation: matcher.match.assert_awaited_once_with(info=entry, ignore_conditions=["123", "Primary", "*"]) @pytest.mark.asyncio - async def test_process_playlist_preserves_only_parent_ignore_conditions(self) -> None: + async def test_playlist_keeps_parent_ignore(self) -> None: captured: dict[str, object] = {} class FakeItem: diff --git a/app/tests/test_datastore.py b/app/tests/test_datastore.py index dfb6bed3..d44120d0 100644 --- a/app/tests/test_datastore.py +++ b/app/tests/test_datastore.py @@ -218,7 +218,7 @@ class TestDataStore: assert ok is True @pytest.mark.asyncio - async def test_bulk_delete_by_status_drops_matching_cached_items(self) -> None: + async def test_bulk_delete_status_cache(self) -> None: connection = Mock() connection.bulk_delete_by_status = AsyncMock(return_value=2) store = DataStore(StoreType.HISTORY, connection) @@ -243,7 +243,7 @@ class TestDataStore: assert pending._id in store._dict @pytest.mark.asyncio - async def test_get_item_returns_none_when_no_kwargs(self) -> None: + async def test_get_item_no_filters(self) -> None: """Test that get_item returns None when no kwargs provided.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -319,7 +319,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_get_item_returns_none_when_no_match(self) -> None: + async def test_get_item_miss(self) -> None: """Test that get_item returns None when no attributes match.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -366,7 +366,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_get_item_returns_first_match(self) -> None: + async def test_get_item_first_match(self) -> None: """Test that get_item returns the first matching item when multiple match.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -689,7 +689,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_has_downloads_with_no_eligible_downloads(self) -> None: + async def test_has_downloads_ineligible(self) -> None: """Test has_downloads returns False when no downloads are eligible.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -708,7 +708,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_get_next_download_returns_none_when_empty(self) -> None: + async def test_next_download_empty(self) -> None: """Test get_next_download returns None when no eligible downloads.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -740,7 +740,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_update_store_item_removes_datetime_field(self) -> None: + async def test_update_item_drops_dt(self) -> None: """Test that _update_store_item removes datetime field before storage.""" db = await make_db() store = DataStore(StoreType.QUEUE, db) @@ -760,7 +760,7 @@ class TestDataStore: await db.close() @pytest.mark.asyncio - async def test_update_store_item_removes_live_in_when_finished(self) -> None: + async def test_update_item_drops_live_in(self) -> None: """Test that _update_store_item removes live_in field when status is finished.""" store = await make_store_async(StoreType.QUEUE) conn = store._connection @@ -781,7 +781,7 @@ class TestDataStore: await store._connection.close() @pytest.mark.asyncio - async def test_update_store_item_keeps_live_in_when_not_finished(self) -> None: + async def test_update_item_keeps_live_in(self) -> None: """Test that _update_store_item keeps live_in field when status is not finished.""" store = await make_store_async(StoreType.QUEUE) conn = store._connection diff --git a/app/tests/test_download.py b/app/tests/test_download.py index f7731fc0..53501413 100644 --- a/app/tests/test_download.py +++ b/app/tests/test_download.py @@ -310,8 +310,9 @@ class TestDownloadFlow: ) class FakeYTDLP: - def __init__(self, params): + def __init__(self, params, enable_custom_outtmpl=False): self.params = params + self.enable_custom_outtmpl = enable_custom_outtmpl self._download_retcode = 0 self._interrupted = False @@ -326,7 +327,7 @@ class TestDownloadFlow: assert queue.items[0]["download_skipped"] is True assert queue.items[1]["download_skipped"] is True - def test_download_resets_sigint_handler_in_worker(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_download_resets_sigint(self, monkeypatch: pytest.MonkeyPatch) -> None: class Cfg: debug = False ytdlp_debug = False @@ -414,9 +415,7 @@ class TestDownloadFlow: assert ydl._interrupted is True ydl.to_screen.assert_called_once_with("[info] Interrupt received, exiting cleanly...") - def test_download_prefers_real_playlist_extras_over_placeholder_preinfo( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: + def test_playlist_extras(self, monkeypatch: pytest.MonkeyPatch) -> None: class Cfg: debug = False ytdlp_debug = False @@ -605,9 +604,7 @@ class TestDownloadFlow: assert download.info.filename == "video.mp4", "Final filename should be set from status update" @pytest.mark.asyncio - async def test_live_cancelled_download_drains_final_status_updates( - self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path - ) -> None: + async def test_live_cancel_drains_final(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: class Cfg: debug = False ytdlp_debug = False @@ -710,7 +707,7 @@ class TestDownloadFlow: assert download.info.filename == "live.mp4", "Finalized live filename should be preserved" @pytest.mark.asyncio - async def test_regular_cancelled_download_skips_live_drain_path(self, monkeypatch: pytest.MonkeyPatch) -> None: + async def test_regular_cancel_skips_drain(self, monkeypatch: pytest.MonkeyPatch) -> None: class Cfg: debug = False ytdlp_debug = False @@ -784,7 +781,7 @@ class TestDownloadSpawnPickling: def setup_method(self): EventBus._reset_singleton() - def test_spawn_pickling_ignores_local_event_listener(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + def test_spawn_pickling_ignores_listener(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: class Cfg: debug = False ytdlp_debug = False @@ -834,7 +831,7 @@ class TestDownloadSpawnPickling: class TestTempManager: - def test_create_temp_path_when_disabled(self) -> None: + def test_create_temp_path_disabled(self) -> None: info = make_item() logger = logging.getLogger("test") tm = TempManager(info, "/tmp", temp_disabled=True, temp_keep=False, logger=logger) @@ -843,7 +840,7 @@ class TestTempManager: assert result is None, "Should return None when temp_disabled is True" assert tm.temp_path is None, "temp_path should remain None when disabled" - def test_create_temp_path_when_no_temp_dir(self) -> None: + def test_create_temp_path_no_dir(self) -> None: info = make_item() logger = logging.getLogger("test") tm = TempManager(info, None, temp_disabled=False, temp_keep=False, logger=logger) @@ -873,7 +870,7 @@ class TestTempManager: path2 = tm2.create_temp_path() assert path1 == path2, "Same download ID should produce same temp path" - def test_delete_temp_when_disabled(self, tmp_path: Path) -> None: + def test_delete_temp_disabled(self, tmp_path: Path) -> None: info = make_item() logger = logging.getLogger("test") tm = TempManager(info, str(tmp_path), temp_disabled=True, temp_keep=False, logger=logger) @@ -883,7 +880,7 @@ class TestTempManager: tm.delete_temp() assert tm.temp_path.exists(), "Should not delete when temp_disabled is True" - def test_delete_temp_when_temp_keep_enabled(self, tmp_path: Path) -> None: + def test_delete_temp_keep(self, tmp_path: Path) -> None: info = make_item() logger = logging.getLogger("test") tm = TempManager(info, str(tmp_path), temp_disabled=False, temp_keep=True, logger=logger) @@ -893,12 +890,13 @@ class TestTempManager: tm.delete_temp() assert tm.temp_path.exists(), "Should not delete when temp_keep is True" - def test_delete_temp_when_no_temp_path(self) -> None: + def test_delete_temp_no_path(self) -> None: info = make_item() logger = logging.getLogger("test") tm = TempManager(info, "/tmp", temp_disabled=False, temp_keep=False, logger=logger) tm.delete_temp() + assert tm.temp_path is None, "temp_path should stay unset" def test_delete_temp_keeps_partial_download(self, tmp_path: Path) -> None: info = make_item() @@ -963,7 +961,7 @@ class TestProcessManager: assert pm.cancel_event.is_set() is False, "Should clear stale cancel events before starting" assert "download-test-id" == proc.name, "Process name should include download ID" - def test_started_returns_true_when_process_created(self) -> None: + def test_started(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) @@ -972,13 +970,13 @@ class TestProcessManager: pm.create_process(lambda: None) assert pm.started() is True, "Should return True after process created" - def test_running_returns_false_when_no_process(self) -> None: + def test_running_no_process(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) assert pm.running() is False, "Should return False when no process" - def test_is_cancelled_returns_false_by_default(self) -> None: + def test_is_cancelled_default(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) @@ -993,7 +991,7 @@ class TestProcessManager: result = pm.cancel() assert pm.is_cancelled() is True, "Should mark as cancelled" - def test_cancel_returns_false_when_not_started(self) -> None: + def test_cancel_not_started(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) @@ -1001,7 +999,7 @@ class TestProcessManager: assert result is False, "Should return False when process not started" assert pm.is_cancelled() is False, "Should not mark as cancelled when not started" - def test_kill_returns_false_when_not_running(self) -> None: + def test_kill_not_running(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) @@ -1024,7 +1022,7 @@ class TestProcessManager: mock_kill.assert_called_once_with(12345, signal.SIGUSR1) assert result is True, "Should return True when process killed successfully" - def test_kill_uses_live_cancel_event_and_longer_graceful_timeout(self) -> None: + def test_kill_live_uses_event(self) -> None: logger = logging.getLogger("test") pm_live = ProcessManager("test-id", is_live=True, logger=logger) pm_regular = ProcessManager("test-id", is_live=False, logger=logger) @@ -1064,7 +1062,7 @@ class TestProcessManager: mock_wait.assert_called_once_with(pm_regular.proc, 5) @pytest.mark.asyncio - async def test_close_returns_false_when_not_started(self) -> None: + async def test_close_not_started(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) @@ -1072,7 +1070,7 @@ class TestProcessManager: assert result is False, "Should return False when process not started" @pytest.mark.asyncio - async def test_close_returns_false_when_cancel_in_progress(self) -> None: + async def test_close_during_cancel(self) -> None: logger = logging.getLogger("test") pm = ProcessManager("test-id", is_live=False, logger=logger) pm.proc = Mock() @@ -1117,7 +1115,7 @@ class TestStatusTracker: assert st.final_update is False, "Should initialize final_update as False" @pytest.mark.asyncio - async def test_process_status_update_ignores_invalid_id(self, mock_config: dict) -> None: + async def test_status_ignores_bad_id(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = {"id": "wrong-id", "status": "downloading"} @@ -1125,7 +1123,7 @@ class TestStatusTracker: assert st.info.status != "downloading", "Should not update status for wrong ID" @pytest.mark.asyncio - async def test_process_status_update_ignores_short_status(self, mock_config: dict) -> None: + async def test_status_ignores_short(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = {"id": "test-id"} @@ -1140,7 +1138,7 @@ class TestStatusTracker: assert st.info.status == "downloading", "Should update info status" @pytest.mark.asyncio - async def test_process_status_update_sets_download_skipped(self, mock_config: dict) -> None: + async def test_status_sets_skipped(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = {"id": "test-id", "status": "downloading", "download_skipped": True} @@ -1156,7 +1154,7 @@ class TestStatusTracker: assert st.tmpfilename == "/tmp/file.part", "Should update tmpfilename" @pytest.mark.asyncio - async def test_process_status_update_calculates_percent(self, mock_config: dict) -> None: + async def test_status_sets_percent(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = { "id": "test-id", @@ -1171,7 +1169,7 @@ class TestStatusTracker: assert st.info.percent == 50.0, "Should calculate percent correctly" @pytest.mark.asyncio - async def test_process_status_update_uses_estimated_total(self, mock_config: dict) -> None: + async def test_status_uses_estimate(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = { "id": "test-id", @@ -1185,7 +1183,7 @@ class TestStatusTracker: assert st.info.percent == 30.0, "Should calculate percent from estimate" @pytest.mark.asyncio - async def test_process_status_update_handles_zero_division(self, mock_config: dict) -> None: + async def test_status_percent(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = { "id": "test-id", @@ -1198,7 +1196,7 @@ class TestStatusTracker: assert st.info.percent == 50.0, "Should calculate percent correctly with valid total" @pytest.mark.asyncio - async def test_process_status_update_sets_speed_and_eta(self, mock_config: dict) -> None: + async def test_status_sets_speed_eta(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) status = {"id": "test-id", "status": "downloading", "speed": 1024000, "eta": 60} @@ -1257,7 +1255,7 @@ class TestStatusTracker: assert st.final_update is True, "Should stop draining after final update" @pytest.mark.asyncio - async def test_drain_queue_handles_errors_gracefully(self, mock_config: dict) -> None: + async def test_drain_queue_skips_invalid(self, mock_config: dict) -> None: queue = DummyQueue() queue.put({"id": "test-id", "status": "downloading"}) queue.put(None) @@ -1266,8 +1264,9 @@ class TestStatusTracker: st = StatusTracker(**config) await st.drain_queue(max_iterations=5) + assert st.info.status == "downloading", "valid updates should still be processed" - def test_cancel_update_task_cancels_running_task(self, mock_config: dict) -> None: + def test_cancel_update_task(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) st.update_task = Mock() st.update_task.done = Mock(return_value=False) @@ -1276,10 +1275,11 @@ class TestStatusTracker: st.cancel_update_task() st.update_task.cancel.assert_called_once() - def test_cancel_update_task_handles_no_task(self, mock_config: dict) -> None: + def test_cancel_update_task_noop(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) st.cancel_update_task() + assert st.update_task is None, "missing tasks should be ignored" def test_put_terminator_adds_to_queue(self, mock_config: dict) -> None: queue = DummyQueue() @@ -1315,7 +1315,7 @@ class TestQueueManager: assert status[item.info._id] == "ok", "Running cancel should still report success" @pytest.mark.asyncio - async def test_cancel_running_regular_item_closes_immediately(self) -> None: + async def test_cancel_regular_closes(self) -> None: queue_manager = object.__new__(DownloadQueue) queue_manager.queue = Mock() queue_manager.done = Mock() @@ -1337,7 +1337,7 @@ class TestQueueManager: assert status[item.info._id] == "ok", "Regular running cancel should still report success" @pytest.mark.asyncio - async def test_clear_flushes_history_deletes_before_returning(self) -> None: + async def test_clear_flushes_history(self) -> None: queue_manager = object.__new__(DownloadQueue) queue_manager.config = Mock(remove_files=False, download_path="/tmp") queue_manager._notify = Mock() @@ -1363,7 +1363,7 @@ class TestQueueManager: assert status[item.info._id] == "ok", "Clear should still report success after flushing deletes" @pytest.mark.asyncio - async def test_clear_bulk_uses_bulk_delete_and_aggregate_notification(self) -> None: + async def test_clear_bulk_notifies(self) -> None: queue_manager = object.__new__(DownloadQueue) queue_manager.config = Mock(remove_files=False, download_path="/tmp") queue_manager._notify = Mock() @@ -1393,7 +1393,7 @@ class TestQueueManager: assert queue_manager._notify.emit.call_args.kwargs["data"] == {"ids": ["done-id-1", "done-id-2"], "count": 2} @pytest.mark.asyncio - async def test_clear_by_status_uses_status_fetch_before_bulk_delete(self) -> None: + async def test_clear_status_fetches(self) -> None: queue_manager = object.__new__(DownloadQueue) queue_manager.config = Mock(remove_files=False, download_path="/tmp") queue_manager._notify = Mock() @@ -1413,7 +1413,7 @@ class TestQueueManager: assert queue_manager._notify.emit.call_args.kwargs["data"] == {"count": 1, "status": "finished"} @pytest.mark.asyncio - async def test_clear_by_status_with_file_removal_fetches_matching_items(self) -> None: + async def test_clear_status_files_fetch(self) -> None: queue_manager = object.__new__(DownloadQueue) queue_manager.config = Mock(remove_files=True, download_path="/tmp") queue_manager._notify = Mock() @@ -1437,7 +1437,7 @@ class TestQueueManager: class TestPoolManager: @pytest.mark.asyncio - async def test_cancelled_entry_with_final_file_stays_finished(self, monkeypatch: pytest.MonkeyPatch) -> None: + async def test_cancelled_file_stays_finished(self, monkeypatch: pytest.MonkeyPatch) -> None: emitted_events: list[str] = [] class EB: @@ -1482,9 +1482,7 @@ class TestPoolManager: done_store.put.assert_awaited_once_with(entry) @pytest.mark.asyncio - async def test_cancelled_regular_entry_with_final_file_stays_cancelled( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: + async def test_cancelled_regular_file_stays(self, monkeypatch: pytest.MonkeyPatch) -> None: emitted_events: list[str] = [] class EB: diff --git a/app/tests/test_download_utils.py b/app/tests/test_download_utils.py index d349acfa..75cb530b 100644 --- a/app/tests/test_download_utils.py +++ b/app/tests/test_download_utils.py @@ -40,7 +40,7 @@ class TestPathUtilities: result = safe_relative_path(file, base, fallback) assert "video.mp4" == result, "Should use fallback path" - def test_safe_relative_path_no_fallback_returns_absolute(self) -> None: + def test_safe_rel_no_fallback(self) -> None: base = Path("/wrong/path") file = Path("/downloads/video.mp4") result = safe_relative_path(file, base) @@ -73,13 +73,13 @@ class TestPathUtilities: class TestProcessUtilities: - def test_wait_for_process_with_timeout_completes_immediately(self) -> None: + def test_wait_timeout_done(self) -> None: proc = Mock() proc.is_alive = Mock(return_value=False) result = wait_for_process_with_timeout(proc, timeout=1.0) assert result is True, "Should return True when process terminates immediately" - def test_wait_for_process_with_timeout_completes_after_delay(self) -> None: + def test_wait_timeout_delay(self) -> None: proc = Mock() call_count = [0] @@ -123,7 +123,7 @@ class TestConfigUtilities: result = parse_extractor_limit("youtube", default_limit=5, max_workers=10) assert 10 == result, "Should cap at max_workers" - def test_parse_extractor_limit_invalid_env_not_digit(self) -> None: + def test_parse_limit_nondigit(self) -> None: logger = logging.getLogger("test") with patch.dict(os.environ, {"YTP_MAX_WORKERS_FOR_YOUTUBE": "abc"}): result = parse_extractor_limit("youtube", default_limit=5, max_workers=10, logger=logger) @@ -138,11 +138,11 @@ class TestConfigUtilities: result = parse_extractor_limit("youtube", default_limit=5, max_workers=10) assert 5 == result, "Should use default when no env var set" - def test_parse_extractor_limit_respects_max_on_default(self) -> None: + def test_parse_limit_default_max(self) -> None: result = parse_extractor_limit("youtube", default_limit=15, max_workers=10) assert 10 == result, "Should cap default at max_workers" - def test_parse_extractor_limit_logs_warning_on_invalid(self) -> None: + def test_parse_limit_warns(self) -> None: logger = Mock() with patch.dict(os.environ, {"YTP_MAX_WORKERS_FOR_YOUTUBE": "invalid"}): parse_extractor_limit("youtube", default_limit=5, max_workers=10, logger=logger) @@ -195,7 +195,7 @@ class TestDataUtilities: assert "custom_field" not in result["info_dict"], "Should exclude custom field" assert "123" == result["info_dict"]["id"], "Should include id" - def test_create_debug_safe_dict_filters_none_and_functions(self) -> None: + def test_debug_safe_filters_none(self) -> None: data = { "status": "downloading", "info_dict": {"id": "123", "none_value": None, "lambda_value": lambda: None, "title": "Video"}, @@ -212,7 +212,7 @@ class TestDataUtilities: class TestStateUtilities: - def test_is_download_stale_terminal_status_finished(self) -> None: + def test_stale_finished(self) -> None: result = is_download_stale( started_time=int(time.time()) - 500, current_status="finished", is_running=False, auto_start=True ) @@ -224,19 +224,19 @@ class TestStateUtilities: ) assert result is False, "Error downloads are never stale" - def test_is_download_stale_terminal_status_cancelled(self) -> None: + def test_stale_cancelled(self) -> None: result = is_download_stale( started_time=int(time.time()) - 500, current_status="cancelled", is_running=False, auto_start=True ) assert result is False, "Cancelled downloads are never stale" - def test_is_download_stale_terminal_status_downloading(self) -> None: + def test_stale_downloading(self) -> None: result = is_download_stale( started_time=int(time.time()) - 500, current_status="downloading", is_running=False, auto_start=True ) assert result is False, "Downloading status is never stale" - def test_is_download_stale_terminal_status_postprocessing(self) -> None: + def test_stale_postprocessing(self) -> None: result = is_download_stale( started_time=int(time.time()) - 500, current_status="postprocessing", is_running=False, auto_start=True ) @@ -303,7 +303,7 @@ class TestTaskExceptionHandling: logger.error.assert_not_called() @pytest.mark.asyncio - async def test_handle_task_exception_ignores_successful(self) -> None: + async def test_task_exception_success(self) -> None: logger = Mock() async def successful_task(): diff --git a/app/tests/test_encoder.py b/app/tests/test_encoder.py index 163b72d6..68f74746 100644 --- a/app/tests/test_encoder.py +++ b/app/tests/test_encoder.py @@ -57,7 +57,7 @@ class TestEncoder: result = self.encoder.default(obj) assert result == {"name": "test", "value": 42} - def test_object_without_dict_fallback_to_default(self): + def test_object_default(self): """Test that objects without __dict__ fall back to default JSONEncoder.""" # This should raise TypeError since complex is not JSON serializable with pytest.raises(TypeError): diff --git a/app/tests/test_events.py b/app/tests/test_events.py index 6e756d1c..f820ea09 100644 --- a/app/tests/test_events.py +++ b/app/tests/test_events.py @@ -460,7 +460,7 @@ class TestEvent: assert f"handler3_{i}" in results @pytest.mark.asyncio - async def test_mixed_sync_async_handlers_execution_order(self): + async def test_mixed_handlers_order(self): """Test that mixed sync/async handlers execute properly without blocking.""" bus = EventBus() execution_times = [] @@ -511,7 +511,7 @@ class TestEvent: class TestEventListener: """Test the EventListener class.""" - def test_event_listener_creation_with_sync_callback(self): + def test_listener_sync_init(self): """Test creating EventListener with synchronous callback.""" def sync_callback(event, name, **kwargs): # noqa: ARG001 @@ -523,7 +523,7 @@ class TestEventListener: assert listener.call_back == sync_callback assert listener.is_coroutine is False - def test_event_listener_creation_with_async_callback(self): + def test_listener_async_init(self): """Test creating EventListener with asynchronous callback.""" async def async_callback(event, name, **kwargs): # noqa: ARG001 diff --git a/app/tests/test_history_routes.py b/app/tests/test_history_routes.py index ddefce14..9b7faf6b 100644 --- a/app/tests/test_history_routes.py +++ b/app/tests/test_history_routes.py @@ -1,12 +1,16 @@ import json +from pathlib import Path +from types import SimpleNamespace from typing import Any from unittest.mock import AsyncMock, Mock import pytest -from app.library.encoder import Encoder from app.library.DataStore import StoreType -from app.routes.api.history import items_delete +from app.library.ItemDTO import ItemDTO +from app.library.encoder import Encoder +from app.routes.api.history import item_rename, items_delete +from app.tests.helpers import temporary_test_dir class _FakeRequest: @@ -14,13 +18,41 @@ class _FakeRequest: self._payload = payload or {} self.query: dict[str, str] = {} self.match_info: dict[str, str] = {} + self.body_exists = bool(payload) async def json(self) -> dict[str, Any]: return self._payload +def _make_download( + *, + filename: str | None = None, + folder: str = "", + download_dir: str | None = None, + status: str = "finished", +) -> SimpleNamespace: + base_dir = download_dir or "/downloads" + original_post_init = ItemDTO.__post_init__ + ItemDTO.__post_init__ = lambda self: None + + try: + item = ItemDTO( + id="test-id", + title="Test Video", + url="https://example.com/watch?v=test-id", + folder=folder, + status=status, + filename=filename, + download_dir=base_dir, + ) + finally: + ItemDTO.__post_init__ = original_post_init + + return SimpleNamespace(info=item) + + @pytest.mark.asyncio -async def test_items_delete_uses_bulk_history_status_clear() -> None: +async def test_items_delete_status() -> None: request = _FakeRequest(payload={"type": StoreType.HISTORY.value, "status": "finished,skip", "remove_file": False}) queue = Mock() queue.clear_by_status = AsyncMock(return_value={"deleted": 12}) @@ -35,7 +67,7 @@ async def test_items_delete_uses_bulk_history_status_clear() -> None: @pytest.mark.asyncio -async def test_items_delete_uses_bulk_history_id_clear() -> None: +async def test_items_delete_ids() -> None: request = _FakeRequest(payload={"type": StoreType.HISTORY.value, "ids": ["a", "b"], "remove_file": False}) queue = Mock() queue.clear_bulk = AsyncMock(return_value={"deleted": 2}) @@ -47,3 +79,112 @@ async def test_items_delete_uses_bulk_history_id_clear() -> None: queue.clear_bulk.assert_awaited_once_with(["a", "b"], remove_file=False) body = json.loads(response.body.decode("utf-8")) assert body == {"items": {}, "deleted": 2} + + +@pytest.mark.asyncio +async def test_item_rename_needs_name() -> None: + request = _FakeRequest(payload={}) + request.match_info["id"] = "item-1" + queue = SimpleNamespace( + done=SimpleNamespace(get_by_id=AsyncMock(return_value=_make_download(filename="video.mp4"))) + ) + encoder = Encoder() + notify = Mock() + config = SimpleNamespace(download_path="/downloads") + + response = await item_rename(request, queue, encoder, notify, config) + + assert response.status == 400 + body = json.loads(response.body.decode("utf-8")) + assert body == {"error": "no data provided."} + + +@pytest.mark.asyncio +async def test_item_rename_missing() -> None: + request = _FakeRequest(payload={"new_name": "renamed.mp4"}) + request.match_info["id"] = "missing" + queue = SimpleNamespace(done=SimpleNamespace(get_by_id=AsyncMock(return_value=None))) + encoder = Encoder() + notify = Mock() + config = SimpleNamespace(download_path="/downloads") + + response = await item_rename(request, queue, encoder, notify, config) + + assert response.status == 404 + body = json.loads(response.body.decode("utf-8")) + assert body == {"error": "item 'missing' not found."} + + +@pytest.mark.asyncio +async def test_item_rename_needs_file() -> None: + request = _FakeRequest(payload={"new_name": "renamed.mp4"}) + request.match_info["id"] = "item-1" + item = _make_download(filename="video.mp4") + queue = SimpleNamespace(done=SimpleNamespace(get_by_id=AsyncMock(return_value=item))) + encoder = Encoder() + notify = Mock() + config = SimpleNamespace(download_path="/downloads") + + item.info.get_file = lambda download_path=None: None + + response = await item_rename(request, queue, encoder, notify, config) + + assert response.status == 400 + body = json.loads(response.body.decode("utf-8")) + assert body == {"error": "item has no downloaded file."} + + +@pytest.mark.asyncio +async def test_item_rename_sidecars() -> None: + with temporary_test_dir("history-rename") as temp_dir: + media = temp_dir / "video.mp4" + subtitle = temp_dir / "video.en.srt" + media.write_text("video") + subtitle.write_text("subtitle") + + request = _FakeRequest(payload={"new_name": "renamed.mp4"}) + request.match_info["id"] = "item-1" + item = _make_download(filename="video.mp4", download_dir=str(temp_dir)) + item.info._id = "item-1" + queue = SimpleNamespace(done=SimpleNamespace(get_by_id=AsyncMock(return_value=item), put=AsyncMock())) + encoder = Encoder() + notify = Mock() + config = SimpleNamespace(download_path=str(temp_dir)) + + response = await item_rename(request, queue, encoder, notify, config) + + assert response.status == 200 + body = json.loads(response.body.decode("utf-8")) + assert body["filename"] == "renamed.mp4" + assert item.info.filename == "renamed.mp4" + assert (temp_dir / "renamed.mp4").exists() + assert (temp_dir / "renamed.en.srt").exists() + assert not media.exists() + assert not subtitle.exists() + queue.done.put.assert_awaited_once_with(item, no_notify=True) + notify.emit.assert_called_once() + + +@pytest.mark.asyncio +async def test_item_rename_conflict() -> None: + with temporary_test_dir("history-rename-conflict") as temp_dir: + media = temp_dir / "video.mp4" + conflict = temp_dir / "renamed.mp4" + media.write_text("video") + conflict.write_text("existing") + + request = _FakeRequest(payload={"new_name": "renamed.mp4"}) + request.match_info["id"] = "item-1" + item = _make_download(filename="video.mp4", download_dir=str(temp_dir)) + queue = SimpleNamespace(done=SimpleNamespace(get_by_id=AsyncMock(return_value=item), put=AsyncMock())) + encoder = Encoder() + notify = Mock() + config = SimpleNamespace(download_path=str(temp_dir)) + + response = await item_rename(request, queue, encoder, notify, config) + + assert response.status == 409 + body = json.loads(response.body.decode("utf-8")) + assert body == {"error": "Destination 'renamed.mp4' already exists"} + queue.done.put.assert_not_awaited() + notify.emit.assert_not_called() diff --git a/app/tests/test_httpx_client.py b/app/tests/test_httpx_client.py index 729d8ccf..d9df6a9b 100644 --- a/app/tests/test_httpx_client.py +++ b/app/tests/test_httpx_client.py @@ -356,7 +356,7 @@ class TestAsyncClient: assert isinstance(client._transport, CFAsyncTransport) @pytest.mark.asyncio - async def test_async_client_cf_disabled(self): + async def test_async_client_custom_cf_off(self): """Test creating async client with CF disabled.""" async with async_client(enable_cf=False) as client: assert isinstance(client, httpx.AsyncClient) @@ -381,7 +381,7 @@ class TestAsyncClient: assert client._transport.base is custom @pytest.mark.asyncio - async def test_async_client_custom_transport_cf_disabled(self): + async def test_async_client_cf_disabled(self): """Test creating async client with custom transport and CF disabled.""" custom = httpx.AsyncHTTPTransport() async with async_client(enable_cf=False, transport=custom) as client: @@ -400,7 +400,7 @@ class TestSyncClient: assert isinstance(client._transport, CFTransport) client.close() - def test_sync_client_cf_disabled(self): + def test_sync_client_custom_cf_off(self): """Test creating sync client with CF disabled.""" client = sync_client(enable_cf=False) assert isinstance(client, httpx.Client) @@ -425,7 +425,7 @@ class TestSyncClient: assert client._transport.base is custom client.close() - def test_sync_client_custom_transport_cf_disabled(self): + def test_sync_client_cf_disabled(self): """Test creating sync client with custom transport and CF disabled.""" custom = httpx.HTTPTransport() client = sync_client(enable_cf=False, transport=custom) diff --git a/app/tests/test_itemdto.py b/app/tests/test_itemdto.py index 0358d57d..aef8ff14 100644 --- a/app/tests/test_itemdto.py +++ b/app/tests/test_itemdto.py @@ -43,7 +43,7 @@ class TestItemFormatAndBasics: assert item.cli == "--embed-metadata" @patch("app.features.presets.service.Presets.get_instance") - def test_format_raises_for_missing_url_and_invalid_preset(self, mock_presets_get): + def test_format_bad_input(self, mock_presets_get): # Missing url with pytest.raises(ValueError, match="url param is required"): Item.format({}) @@ -113,7 +113,7 @@ class TestItemDTO: @patch("app.library.ItemDTO.get_archive_id") @patch("app.library.ItemDTO.YTDLPOpts") @patch("app.library.ItemDTO.archive_read") - def test_post_init_does_not_infer_download_skipped_flag(self, mock_read, mock_opts, mock_get_id): + def test_post_init_keeps_skipped(self, mock_read, mock_opts, mock_get_id): mock_get_id.return_value = {"archive_id": "arch", "id": "arch", "ie_key": "YT"} mock_opts.get_instance.return_value.preset.return_value = mock_opts.get_instance.return_value mock_opts.get_instance.return_value.add_cli.return_value = mock_opts.get_instance.return_value @@ -128,7 +128,7 @@ class TestItemDTO: assert dto.download_skipped is False @patch("app.library.ItemDTO.archive_read") - def test_serialize_triggers_archive_status_when_finished(self, mock_read): + def test_serialize_archives_finished(self, mock_read): # Given a finished item with archive info dto = ItemDTO(id="vid", title="t", url="u", folder="f") dto.archive_id = "arch" @@ -143,7 +143,7 @@ class TestItemDTO: assert key not in data @patch("app.library.ItemDTO.YTDLPOpts") - def test_serialize_does_not_recompute_download_skipped(self, mock_opts): + def test_serialize_keeps_skipped(self, mock_opts): mock_opts.get_instance.return_value.preset.return_value = mock_opts.get_instance.return_value mock_opts.get_instance.return_value.add_cli.return_value = mock_opts.get_instance.return_value mock_opts.get_instance.return_value.get_all.return_value = {} @@ -299,7 +299,7 @@ class TestItemDTO: assert result is expected_sidecar assert dto.sidecar is expected_sidecar - def test_get_file_sidecar_returns_existing_when_no_file(self): + def test_sidecar_no_file(self): with patch.object(ItemDTO, "__post_init__", lambda _: None): dto = ItemDTO(id="sidecar-none", title="Title", url="u", folder="f") @@ -317,7 +317,7 @@ class TestItemDTO: assert result is existing assert dto.sidecar is existing - def test_get_preset_returns_preset_instance(self): + def test_get_preset_hit(self): """Test ItemDTO.get_preset returns the Preset instance.""" from app.features.presets.schemas import Preset @@ -335,7 +335,7 @@ class TestItemDTO: assert result is mock_preset assert result.name == "test-preset" - def test_get_preset_uses_default_when_no_preset_set(self): + def test_get_preset_default(self): """Test ItemDTO.get_preset uses 'default' when preset is empty.""" from app.features.presets.schemas import Preset @@ -352,7 +352,7 @@ class TestItemDTO: mock_presets.return_value.get.assert_called_once_with("default") assert result is mock_preset - def test_get_preset_returns_none_when_not_found(self): + def test_get_preset_miss(self): """Test ItemDTO.get_preset returns None when preset not found.""" with patch.object(ItemDTO, "__post_init__", lambda _: None): dto = ItemDTO(id="vid", title="t", url="u", folder="f", preset="nonexistent") @@ -376,7 +376,7 @@ class TestItemAddExtras: assert item.extras["key1"] == "value1" - def test_add_extras_when_extras_is_none(self): + def test_add_extras_none(self): """Test adding extras when extras is None.""" item = Item(url="https://example.com") item.extras = None diff --git a/app/tests/test_package_installer.py b/app/tests/test_package_installer.py index e07b736a..62f47ef4 100644 --- a/app/tests/test_package_installer.py +++ b/app/tests/test_package_installer.py @@ -38,7 +38,7 @@ class TestPackages: class TestPackageInstallerInit: - def test_init_with_explicit_path_adds_to_sys_path(self, tmp_path: Path) -> None: + def test_init_adds_sys_path(self, tmp_path: Path) -> None: p = tmp_path / "site" p.mkdir() @@ -58,7 +58,7 @@ class TestPackageInstallerInit: assert installer.user_site.exists() is True assert str(installer.user_site) in sys.path - def test_init_without_path_or_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_init_no_path(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("YTP_CONFIG_PATH", raising=False) installer = PackageInstaller(pkg_path=None) # No user_site is set when no path or env provided @@ -206,7 +206,7 @@ class TestInstallCmd: class TestActionAndCheck: @patch.object(PackageInstaller, "_install_pkg") @patch.object(PackageInstaller, "_get_installed_version") - def test_action_skips_when_same_pinned(self, mock_get_installed, mock_install, tmp_path: Path) -> None: + def test_action_skip_pinned(self, mock_get_installed, mock_install, tmp_path: Path) -> None: inst = PackageInstaller(pkg_path=tmp_path) mock_get_installed.return_value = "1.2.3" # compare_versions normal equality should hold @@ -216,9 +216,7 @@ class TestActionAndCheck: @patch.object(PackageInstaller, "_install_pkg") @patch.object(PackageInstaller, "_get_installed_version") @patch.object(PackageInstaller, "_get_latest_version") - def test_action_upgrade_skip_when_latest( - self, mock_get_latest, mock_get_installed, mock_install, tmp_path: Path - ) -> None: + def test_action_skip_latest(self, mock_get_latest, mock_get_installed, mock_install, tmp_path: Path) -> None: inst = PackageInstaller(pkg_path=tmp_path) mock_get_installed.return_value = "2.0.0" mock_get_latest.return_value = "2.0.0" @@ -228,9 +226,7 @@ class TestActionAndCheck: @patch.object(PackageInstaller, "_install_pkg") @patch.object(PackageInstaller, "_get_installed_version") @patch.object(PackageInstaller, "_get_latest_version") - def test_action_upgrade_runs_when_newer_available( - self, mock_get_latest, mock_get_installed, mock_install, tmp_path: Path - ) -> None: + def test_action_upgrade_newer(self, mock_get_latest, mock_get_installed, mock_install, tmp_path: Path) -> None: inst = PackageInstaller(pkg_path=tmp_path) mock_get_installed.return_value = "1.0.0" mock_get_latest.return_value = "1.1.0" @@ -239,13 +235,13 @@ class TestActionAndCheck: @patch.object(PackageInstaller, "_install_pkg") @patch.object(PackageInstaller, "_get_installed_version") - def test_action_install_when_not_installed(self, mock_get_installed, mock_install, tmp_path: Path) -> None: + def test_action_install_missing(self, mock_get_installed, mock_install, tmp_path: Path) -> None: inst = PackageInstaller(pkg_path=tmp_path) mock_get_installed.return_value = None inst.action("pkg") mock_install.assert_called_once_with("pkg", version=None) - def test_check_with_no_packages_or_no_user_site(self, tmp_path: Path) -> None: + def test_check_no_packages_or_path(self, tmp_path: Path) -> None: # No packages inst = PackageInstaller(pkg_path=tmp_path) pkgs = Packages(env=None, file=None, upgrade=False) @@ -257,7 +253,7 @@ class TestActionAndCheck: inst2.check(pkgs2) @patch.object(PackageInstaller, "action") - def test_check_calls_action_and_handles_errors(self, mock_action, tmp_path: Path) -> None: + def test_check_runs_all(self, mock_action, tmp_path: Path) -> None: inst = PackageInstaller(pkg_path=tmp_path) pkgs = Packages(env="foo bar", file=None, upgrade=True) @@ -272,3 +268,5 @@ class TestActionAndCheck: # Should not raise inst.check(pkgs) assert mock_action.call_count == 2 + mock_action.assert_any_call("foo", upgrade=True) + mock_action.assert_any_call("bar", upgrade=True) diff --git a/app/tests/test_router.py b/app/tests/test_router.py index ba0913b6..f6ecf280 100644 --- a/app/tests/test_router.py +++ b/app/tests/test_router.py @@ -90,7 +90,7 @@ class TestAddRoute: assert r.method == "POST" assert r.path == "/api/create/" - def test_add_route_socket_without_alias(self) -> None: + def test_add_route_socket_no_alias(self) -> None: async def s(): return "s" @@ -109,7 +109,7 @@ class TestAddRoute: class TestGetters: - def test_get_routes_returns_copy_like_mapping(self) -> None: + def test_get_routes_copy(self) -> None: async def h(): return "x" diff --git a/app/tests/test_scheduler.py b/app/tests/test_scheduler.py index 08c47352..dfa5ef24 100644 --- a/app/tests/test_scheduler.py +++ b/app/tests/test_scheduler.py @@ -184,7 +184,7 @@ class TestScheduler: @pytest.mark.asyncio @patch("app.library.Scheduler.Cron", new=DummyCron) - async def test_attach_registers_shutdown_and_handles_schedule_add_event(self) -> None: + async def test_attach_registers_events(self) -> None: from aiohttp import web app = web.Application() @@ -224,7 +224,7 @@ class TestScheduler: assert kwargs["id"] == "evt-job" @patch("app.library.Scheduler.Cron") - def test_add_executes_function_when_cron_runs(self, cron_patch) -> None: + def test_add_executes_on_start(self, cron_patch) -> None: # Cron stub that auto-runs the function on creation when start=True class AutoRunCron(DummyCron): def __init__( diff --git a/app/tests/test_services.py b/app/tests/test_services.py index 10aa66ad..c2a988cc 100644 --- a/app/tests/test_services.py +++ b/app/tests/test_services.py @@ -75,7 +75,7 @@ class TestServices: assert services.get("service3") == "value3" assert len(services.get_all()) == 3 - def test_get_all_returns_copy(self): + def test_get_all_copy(self): """Test that get_all returns a copy, not the original dict.""" services = Services() services.add("test", "value") diff --git a/app/tests/test_sqlite_store.py b/app/tests/test_sqlite_store.py index 56326fa1..6039ae3f 100644 --- a/app/tests/test_sqlite_store.py +++ b/app/tests/test_sqlite_store.py @@ -32,7 +32,7 @@ def make_item(idx: int, *, status: str = "finished", cli: str = "", download_ski @pytest.mark.asyncio -async def test_sessionmaker_returns_valid_sessionmaker() -> None: +async def test_sessionmaker_ready() -> None: """Test that sessionmaker() returns a working async_sessionmaker.""" SqliteStore._reset_singleton() store = SqliteStore.get_instance(db_path=make_in_memory_db_path("sessionmaker")) @@ -120,7 +120,7 @@ async def test_enqueue_upsert_and_fetch_saved(): @pytest.mark.asyncio -async def test_enqueue_upsert_persists_download_skipped_flag(): +async def test_enqueue_upsert_skipped(): store = await make_store() item = make_item(2, download_skipped=True) @@ -237,7 +237,7 @@ async def test_enqueue_delete_removes_row(): @pytest.mark.asyncio -async def test_enqueue_bulk_delete_returns_count_and_bulk_path(): +async def test_enqueue_bulk_delete(): store = await make_store() items = [make_item(i) for i in range(3)] for itm in items: @@ -258,7 +258,7 @@ async def test_enqueue_bulk_delete_returns_count_and_bulk_path(): @pytest.mark.asyncio -async def test_get_many_by_ids_and_status_and_bulk_delete_by_status(): +async def test_ids_status_bulk_delete(): store = await make_store() finished = [make_item(i, status="finished") for i in range(2)] pending = [make_item(i + 10, status="pending") for i in range(2)] @@ -296,7 +296,7 @@ async def test_get_many_by_ids_and_status_and_bulk_delete_by_status(): @pytest.mark.asyncio -async def test_paginate_out_of_range_returns_last_page(): +async def test_paginate_last_page(): store = await make_store() for i in range(7): await store.enqueue_upsert("history", make_item(i)) @@ -311,7 +311,7 @@ async def test_paginate_out_of_range_returns_last_page(): @pytest.mark.asyncio -async def test_get_item_returns_none_without_kwargs(): +async def test_get_item_no_filters(): store = await make_store() await store.enqueue_upsert("queue", make_item(1)) await store.flush() @@ -387,7 +387,7 @@ async def test_exists_and_get_by_key_and_url(): @pytest.mark.asyncio -async def test_exists_and_get_raise_without_key_or_url(): +async def test_exists_and_get_require_lookup(): store = await make_store() with pytest.raises(KeyError): await store.exists("queue") diff --git a/app/tests/test_static_routes.py b/app/tests/test_static_routes.py index f17dae09..8b40a5c6 100644 --- a/app/tests/test_static_routes.py +++ b/app/tests/test_static_routes.py @@ -34,7 +34,7 @@ def _configure_static_root(static_root: Path) -> None: class TestServeStaticFile: @pytest.mark.asyncio - async def test_nested_document_route_falls_back_to_spa_shell(self, tmp_path: Path) -> None: + async def test_nested_doc_falls_back(self, tmp_path: Path) -> None: config = Config.get_instance() index_file = tmp_path / "index.html" index_file.write_text("root shell", encoding="utf-8") @@ -46,7 +46,7 @@ class TestServeStaticFile: assert response._path == index_file @pytest.mark.asyncio - async def test_generated_nested_index_is_not_preferred_over_root_shell(self, tmp_path: Path) -> None: + async def test_nested_index_uses_root(self, tmp_path: Path) -> None: config = Config.get_instance() root_index = tmp_path / "index.html" nested_index = tmp_path / "docs" / "readme" / "index.html" @@ -82,7 +82,7 @@ class TestServeStaticFile: assert b"missing.js" in body @pytest.mark.asyncio - async def test_missing_asset_with_unknown_suffix_does_not_fall_back(self, tmp_path: Path) -> None: + async def test_missing_unknown_no_fallback(self, tmp_path: Path) -> None: config = Config.get_instance() (tmp_path / "index.html").write_text("root shell", encoding="utf-8") assets_dir = tmp_path / "assets" @@ -103,7 +103,7 @@ class TestServeStaticFile: assert b"missing.abcd123" in body @pytest.mark.asyncio - async def test_symlink_outside_static_root_does_not_resolve(self, tmp_path: Path) -> None: + async def test_symlink_outside_rejected(self, tmp_path: Path) -> None: config = Config.get_instance() (tmp_path / "index.html").write_text("root shell", encoding="utf-8") outside_dir = tmp_path.parent / "outside-static-root" @@ -144,7 +144,7 @@ class TestServeStaticFile: assert b"/api/missing" in body @pytest.mark.asyncio - async def test_dotted_browser_path_returns_not_found(self, tmp_path: Path) -> None: + async def test_dotted_browser_path_404(self, tmp_path: Path) -> None: config = Config.get_instance() (tmp_path / "index.html").write_text("root shell", encoding="utf-8") _configure_static_root(tmp_path) @@ -157,7 +157,7 @@ class TestServeStaticFile: assert response.status == web.HTTPNotFound.status_code assert b"/browser/foo/bar.txt" in body - def test_registers_only_root_and_catch_all_routes(self, tmp_path: Path) -> None: + def test_registers_root_routes(self, tmp_path: Path) -> None: config = Config.get_instance() static_root = tmp_path / "ui-exported" static_root.mkdir() diff --git a/app/tests/test_system_routes.py b/app/tests/test_system_routes.py index f578e248..eefbfda5 100644 --- a/app/tests/test_system_routes.py +++ b/app/tests/test_system_routes.py @@ -121,7 +121,7 @@ class TestSystemLimitsEndpoint: Config._reset_singleton() @pytest.mark.asyncio - async def test_system_limits_returns_user_facing_limits(self): + async def test_system_limits_public(self): config = Config.get_instance() config.max_workers = 10 config.max_workers_per_extractor = 3 @@ -188,7 +188,7 @@ class TestSystemLimitsEndpoint: "available": 3, } - def test_config_reads_prevent_live_premiere_boolean_env(self): + def test_config_reads_live_premiere(self): with patch.dict("os.environ", {"YTP_PREVENT_LIVE_PREMIERE": "false"}, clear=False): Config._reset_singleton() config = Config.get_instance() diff --git a/app/tests/test_terminal_session_manager.py b/app/tests/test_terminal_session_manager.py index f2962db4..236f369a 100644 --- a/app/tests/test_terminal_session_manager.py +++ b/app/tests/test_terminal_session_manager.py @@ -1,10 +1,13 @@ import asyncio import json from pathlib import Path +import time from typing import Any, cast import pytest +from aiohttp import web +from app.library.Scheduler import Scheduler from app.library.Services import Services from app.library.TerminalSessionManager import TerminalSessionManager from app.library.config import Config @@ -13,6 +16,7 @@ from app.routes.api.system import ( cancel_terminal_session, create_terminal_session, get_active_terminal_session, + list_terminal_sessions, get_terminal_session, stream_terminal_session, ) @@ -133,6 +137,7 @@ class _TerminableProc: @pytest.fixture def terminal_setup(tmp_path: Path) -> tuple[Config, TerminalSessionManager, Encoder]: + Scheduler._reset_singleton() Services._reset_singleton() Config._reset_singleton() TerminalSessionManager._reset_singleton() @@ -150,8 +155,18 @@ def terminal_setup(tmp_path: Path) -> tuple[Config, TerminalSessionManager, Enco class TestTerminalSessionRoutes: + def test_attach_registers_cleanup_job(self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder]) -> None: + _config, manager, _encoder = terminal_setup + app = web.Application() + + manager.attach(app) + + scheduler = Scheduler.get_instance() + assert scheduler.has(f"{TerminalSessionManager.__name__}.{TerminalSessionManager.cleanup.__name__}") + assert manager._cleanup_job_id == f"{TerminalSessionManager.__name__}.{TerminalSessionManager.cleanup.__name__}" + @pytest.mark.asyncio - async def test_start_returns_session_metadata_and_active_conflict( + async def test_start_conflict_meta( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -191,7 +206,7 @@ class TestTerminalSessionRoutes: await task @pytest.mark.asyncio - async def test_stream_endpoint_replays_persisted_events_and_resume_cursor( + async def test_stream_replays_resume( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -241,11 +256,11 @@ class TestTerminalSessionRoutes: assert "id: 3" in resumed_payload @pytest.mark.asyncio - async def test_completed_session_expires_after_drain_window( + async def test_completed_expires( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup - manager._drain_ttl = 0.05 + manager._completed_retention = 0.05 await manager.initialize() async def fake_create_subprocess_exec(*_args, **_kwargs): @@ -271,10 +286,130 @@ class TestTerminalSessionRoutes: expired = await manager.get_session(session_id) assert expired is None + assert (manager.root_path / session_id).exists() + + await manager.cleanup() + assert not (manager.root_path / session_id).exists() @pytest.mark.asyncio - async def test_shutdown_interrupts_active_session_and_clears_active_marker( + async def test_list_keeps_recent_done( + self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch + ) -> None: + config, manager, encoder = terminal_setup + manager._completed_retention = 0.2 + manager._drain_ttl = 0.01 + await manager.initialize() + + async def fake_create_subprocess_exec(*_args, **_kwargs): + return _CompletedProc([b"done\n"]) + + monkeypatch.setattr( + "app.library.TerminalSessionManager.asyncio.create_subprocess_exec", fake_create_subprocess_exec + ) + monkeypatch.setattr(manager, "_open_pty", lambda: None) + + start_request = _FakeRequest(payload={"command": "--help"}, can_read_body=True) + start_response = await create_terminal_session(start_request, config, encoder, manager) + session_id = json.loads(start_response.body.decode("utf-8"))["session_id"] + + assert manager._active is not None + await manager._active.task + + await asyncio.sleep(0.03) + + listed_response = await list_terminal_sessions(config, encoder, manager) + listed_payload = json.loads(listed_response.body.decode("utf-8")) + + assert 200 == listed_response.status + assert 1 == len(listed_payload["items"]) + assert session_id == listed_payload["items"][0]["session_id"] + assert "completed" == listed_payload["items"][0]["status"] + assert listed_payload["items"][0]["available_until"] is not None + + persisted = await manager.get_session(session_id) + assert persisted is not None + + await asyncio.sleep(0.19) + + expired = await manager.get_session(session_id) + assert expired is None + assert (manager.root_path / session_id).exists() + + await manager.cleanup() + + assert not (manager.root_path / session_id).exists() + + @pytest.mark.asyncio + async def test_list_orders_newest(self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder]) -> None: + config, manager, encoder = terminal_setup + await manager.initialize() + + first_id = "first" + second_id = "second" + expired_id = "expired" + + for session_id in [first_id, second_id, expired_id]: + session_dir = manager.root_path / session_id + session_dir.mkdir(parents=True, exist_ok=True) + manager._transcript_path(session_id).touch(exist_ok=True) + + manager._write_json( + manager._metadata_path(first_id), + { + "session_id": first_id, + "command": "--first", + "status": "completed", + "created_at": 10.0, + "started_at": 11.0, + "finished_at": 20.0, + "expires_at": time.time() + 60, + "exit_code": 0, + "last_sequence": 2, + }, + ) + manager._write_json( + manager._metadata_path(second_id), + { + "session_id": second_id, + "command": "--second", + "status": "running", + "created_at": 30.0, + "started_at": 31.0, + "finished_at": None, + "expires_at": None, + "exit_code": None, + "last_sequence": 4, + }, + ) + manager._write_json( + manager._metadata_path(expired_id), + { + "session_id": expired_id, + "command": "--expired", + "status": "completed", + "created_at": 1.0, + "started_at": 2.0, + "finished_at": 3.0, + "expires_at": time.time() - 1, + "exit_code": 1, + "last_sequence": 1, + }, + ) + + listed_response = await list_terminal_sessions(config, encoder, manager) + listed_payload = json.loads(listed_response.body.decode("utf-8")) + + assert 200 == listed_response.status + assert [second_id, first_id] == [item["session_id"] for item in listed_payload["items"]] + assert (manager.root_path / expired_id).exists() + + await manager.cleanup() + + assert not (manager.root_path / expired_id).exists() + + @pytest.mark.asyncio + async def test_shutdown_clears_active( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -314,7 +449,7 @@ class TestTerminalSessionRoutes: assert -15 == transcript[-1]["data"]["exitcode"] @pytest.mark.asyncio - async def test_stream_endpoint_emits_keepalive_for_silent_active_session( + async def test_stream_keepalive( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -352,7 +487,7 @@ class TestTerminalSessionRoutes: assert 'data: {"exitcode": 0}' in stream_payload @pytest.mark.asyncio - async def test_cancel_endpoint_interrupts_active_session( + async def test_cancel_active( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -398,7 +533,7 @@ class TestTerminalSessionRoutes: assert -15 == transcript[-1]["data"]["exitcode"] @pytest.mark.asyncio - async def test_cancel_endpoint_returns_conflict_for_inactive_session( + async def test_cancel_inactive_conflict( self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder], monkeypatch: pytest.MonkeyPatch ) -> None: config, manager, encoder = terminal_setup @@ -427,9 +562,7 @@ class TestTerminalSessionRoutes: assert b"not active" in cancel_response.body.lower() @pytest.mark.asyncio - async def test_cancel_endpoint_returns_not_found_for_unknown_session( - self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder] - ) -> None: + async def test_cancel_unknown(self, terminal_setup: tuple[Config, TerminalSessionManager, Encoder]) -> None: config, manager, encoder = terminal_setup await manager.initialize() diff --git a/app/tests/test_test_helpers.py b/app/tests/test_test_helpers.py index 4f52e55e..947a47f8 100644 --- a/app/tests/test_test_helpers.py +++ b/app/tests/test_test_helpers.py @@ -10,14 +10,14 @@ from app.tests.helpers import ( ) -def test_make_test_disk_path_uses_test_run_root() -> None: +def test_disk_path_root() -> None: path = make_test_disk_path("artifacts", "example.txt") assert path.parent.exists() assert path.is_relative_to(get_test_run_root()) -def test_make_test_temp_dir_creates_directory() -> None: +def test_temp_dir_created() -> None: path = make_test_temp_dir("helpers") assert path.exists() @@ -25,7 +25,7 @@ def test_make_test_temp_dir_creates_directory() -> None: assert path.is_relative_to(get_test_run_root()) -def test_tmp_path_runs_under_custom_temp_root(tmp_path: Path) -> None: +def test_tmp_path_root(tmp_path: Path) -> None: expected_root = get_test_run_root() / "pytest" assert tmp_path.is_relative_to(expected_root) diff --git a/app/tests/test_update_checker.py b/app/tests/test_update_checker.py index 25687e68..4cb23699 100644 --- a/app/tests/test_update_checker.py +++ b/app/tests/test_update_checker.py @@ -21,7 +21,7 @@ class TestUpdateChecker: EventBus._reset_singleton() Cache._reset_singleton() - def test_attach_schedules_check_when_enabled(self): + def test_attach_enabled(self): """Test that attach schedules update check when config.check_for_updates is True.""" import asyncio @@ -50,7 +50,7 @@ class TestUpdateChecker: scheduler.remove(checker._job_id) loop.close() - def test_attach_skips_scheduling_when_disabled(self): + def test_attach_disabled(self): """Test that attach skips scheduling when config.check_for_updates is False.""" from app.library.config import Config from app.library.UpdateChecker import UpdateChecker @@ -111,7 +111,7 @@ class TestUpdateChecker: loop.close() @pytest.mark.asyncio - async def test_check_for_updates_skips_when_disabled(self): + async def test_check_for_updates_disabled(self): """Test that check_for_updates skips when config.check_for_updates is False.""" from app.library.config import Config from app.library.UpdateChecker import UpdateChecker @@ -255,7 +255,7 @@ class TestUpdateChecker: @pytest.mark.asyncio @patch("app.library.UpdateChecker.get_async_client") - async def test_check_ytdlp_version_handles_http_error(self, mock_client): + async def test_check_ytdlp_version_http_error(self, mock_client): """Test that yt-dlp check handles HTTP errors gracefully.""" from app.library.config import Config from app.library.UpdateChecker import UpdateChecker diff --git a/app/tests/test_utils.py b/app/tests/test_utils.py index e2375067..ff65a372 100644 --- a/app/tests/test_utils.py +++ b/app/tests/test_utils.py @@ -1,5 +1,6 @@ import asyncio import copy +import importlib import re import shutil import uuid @@ -30,7 +31,6 @@ from app.library.Utils import ( is_private_address, list_folders, load_cookies, - load_modules, merge_dict, move_file, parse_tags, @@ -48,7 +48,7 @@ from app.tests.helpers import make_test_temp_dir, temporary_test_dir class TestTimedLruCache: """Test the timed_lru_cache decorator.""" - def test_timed_lru_cache_basic_functionality(self): + def test_basic(self): """Test that timed_lru_cache caches function results.""" call_count = 0 @@ -332,39 +332,39 @@ class TestCalcDownloadPath: result = calc_download_path(str(self.base_path), None, create_path=False) assert result == str(self.base_path), "Should return base path when folder is None" - def test_calc_download_path_removes_leading_slash(self): + def test_calc_path_strips_leading_slash(self): """Test that leading slash is removed from folder.""" folder = "/test_folder" result = calc_download_path(str(self.base_path), folder, create_path=False) expected = str(self.base_path / "test_folder") assert result == expected, "Should remove leading slash from folder" - def test_calc_download_path_path_traversal_dotdot(self): + def test_calc_path_dotdot(self): """Test path traversal prevention with .. sequences.""" folder = "../outside" with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_path_traversal_nested_dotdot(self): + def test_calc_path_nested_dotdot(self): """Test path traversal prevention with nested .. sequences.""" folder = "safe/../../outside" with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_path_traversal_multiple_dotdot(self): + def test_calc_path_multi_dotdot(self): """Test path traversal prevention with multiple .. sequences.""" folder = "../../../etc/passwd" with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_path_traversal_absolute_path(self): + def test_calc_path_absolute(self): """Test that absolute paths are made safe by removing leading slash.""" folder = "/etc/passwd" result = calc_download_path(str(self.base_path), folder, create_path=False) expected = str(self.base_path / "etc/passwd") assert result == expected, "Should remove leading slash and treat as relative path" - def test_calc_download_path_path_traversal_absolute_with_dotdot(self): + def test_calc_path_absolute_dotdot(self): """Test path traversal with absolute path containing .. sequences.""" folder = "/../../../etc/passwd" with pytest.raises(Exception, match="must resolve inside the base download folder"): @@ -376,7 +376,7 @@ class TestCalcDownloadPath: with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_path_traversal_url_encoded(self): + def test_calc_path_url_encoded(self): """Test path traversal prevention with URL encoded sequences.""" folder = "safe%2F..%2F..%2Funsafe" # safe/../unsafe encoded # This should be handled at a higher level, but let's test it anyway @@ -449,7 +449,7 @@ class TestCalcDownloadPath: with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_carriage_return_attack(self): + def test_calc_path_carriage_return(self): """Test that carriage returns in path traversal are prevented.""" folder = "folder\r../../../etc/passwd" with pytest.raises(Exception, match="must resolve inside the base download folder"): @@ -479,7 +479,7 @@ class TestCalcDownloadPath: with pytest.raises(Exception, match="must resolve inside the base download folder"): calc_download_path(str(self.base_path), folder, create_path=False) - def test_calc_download_path_url_encoded_traversal_safe(self): + def test_calc_path_url_encoded_safe(self): """Test that URL encoded path traversal is treated as literal (safe).""" folder = "folder..%2F..%2F..%2Fetc%2Fpasswd" # ..%2F = ../ encoded result = calc_download_path(str(self.base_path), folder, create_path=False) @@ -494,7 +494,7 @@ class TestCalcDownloadPath: expected = str(self.base_path / folder) assert result == expected, "Backslashes should be treated as literal characters on Unix" - def test_calc_download_path_mixed_separators_attack(self): + def test_calc_path_mixed_separators(self): """Test path traversal with mixed separators.""" folder = "folder/../../../etc/passwd" with pytest.raises(Exception, match="must resolve inside the base download folder"): @@ -520,7 +520,7 @@ class TestCalcDownloadPath: # Clean up shutil.rmtree(sibling_dir, ignore_errors=True) - def test_calc_download_path_symlink_attack_outside(self): + def test_calc_path_symlink_outside(self): """Test that symlinks pointing outside base directory are blocked.""" # Create a symlink pointing outside the base directory outside_dir = Path(self.temp_dir).parent / "outside_target" @@ -539,7 +539,7 @@ class TestCalcDownloadPath: symlink_path.unlink() shutil.rmtree(outside_dir, ignore_errors=True) - def test_calc_download_path_symlink_attack_with_traversal(self): + def test_calc_path_symlink_traversal(self): """Test symlink combined with path traversal.""" # Create a directory outside base outside_dir = Path(self.temp_dir).parent / "target_dir" @@ -563,7 +563,7 @@ class TestCalcDownloadPath: shutil.rmtree(safe_dir, ignore_errors=True) shutil.rmtree(outside_dir, ignore_errors=True) - def test_calc_download_path_symlink_safe_internal(self): + def test_calc_path_symlink_internal(self): """Test that symlinks pointing inside base directory are allowed.""" # Create target directory inside base target_dir = self.base_path / "target" @@ -602,7 +602,7 @@ class TestCalcDownloadPath: expected = str(self.base_path / deep_path) assert result == expected, "Should handle deeply nested paths" - def test_calc_download_path_directory_with_spaces(self): + def test_calc_path_spaces(self): """Test paths with multiple spaces and special spacing.""" folder = "folder with multiple spaces" result = calc_download_path(str(self.base_path), folder, create_path=True) @@ -697,7 +697,7 @@ class TestMergeDict: assert "__builtins__" not in result, "__builtins__ should be filtered out" assert result["normal"] == "value", "Normal values should be preserved" - def test_merge_dict_blocks_multiple_dunder_pollution(self): + def test_merge_dict_blocks_dunders(self): """Test that multiple dangerous dunder attributes are blocked.""" source = { "__class__": "malicious", @@ -863,7 +863,7 @@ class TestMergeDict: assert len(result["data"]) == 5000 assert result["data"] == large_list - def test_merge_dict_list_merging_with_size_limits(self): + def test_merge_dict_list_limits(self): """Test list merging respects size limits.""" source = {"items": list(range(3000))} destination = {"items": list(range(2000, 5000))} # 3000 items @@ -921,7 +921,7 @@ class TestMergeDict: result = merge_dict(source, {}, max_depth=10000, max_list_size=1000000) assert result["a"]["b"]["c"]["data"] == "nested" - def test_merge_dict_limits_with_circular_reference_protection(self): + def test_merge_dict_circular_guard(self): """Test that limits work together with circular reference protection.""" source = {"data": {}} source["data"]["circular"] = source # Create circular reference @@ -1088,9 +1088,7 @@ class TestValidateUuid: def test_wrong_version(self): """Test UUID4 against version 1 check.""" test_uuid = str(uuid.uuid4()) - # Version check is not strict in this function - it may return True for any valid UUID - result = validate_uuid(test_uuid, 1) - assert isinstance(result, bool) # Just check it returns a boolean + assert validate_uuid(test_uuid, 1) is True class TestStripNewline: @@ -1222,14 +1220,13 @@ class TestValidateUrl: def test_validate_url_basic(self): """Test basic URL validation functionality.""" - # Test without actual validation due to missing yarl dependency - # Just check the function exists and handles the missing dependency gracefully - try: - result = validate_url("https://example.com") - assert isinstance(result, bool) - except ModuleNotFoundError: - # Expected when yarl is not installed - assert True + if importlib.util.find_spec("yarl") is None: + with pytest.raises(ModuleNotFoundError): + validate_url("https://example.com") + return + + result = validate_url("https://example.com", allow_internal=True) + assert result is True class TestGetFileSidecar: @@ -1300,32 +1297,36 @@ class TestArgConverter: def test_arg_converter_basic(self): """Test basic arg_converter functionality.""" - try: - result = arg_converter("--quiet --match-filters 'duration<2min' --download-archive archive.txt") - assert isinstance(result, dict) - assert result.get("quiet") is True, "quiet should be True" - assert result.get("download_archive") == "archive.txt" - assert "match_filter" in result, "match_filters should be in result" - except (ModuleNotFoundError, AttributeError, ImportError): - # Expected when yt_dlp is not available or differs - assert True + if importlib.util.find_spec("yt_dlp") is None: + with pytest.raises(ModuleNotFoundError): + arg_converter("--quiet --match-filters 'duration<2min' --download-archive archive.txt") + return + + result = arg_converter("--quiet --match-filters 'duration<2min' --download-archive archive.txt") + assert isinstance(result, dict) + assert result.get("quiet") is True, "quiet should be True" + assert result.get("download_archive") == "archive.txt" + assert "match_filter" in result, "match_filters should be in result" def test_arg_converter_empty_args(self): """Test arg_converter with empty args.""" - try: - result = arg_converter("") - assert isinstance(result, dict) - except (ModuleNotFoundError, AttributeError, ImportError): - assert True + if importlib.util.find_spec("yt_dlp") is None: + with pytest.raises(ModuleNotFoundError): + arg_converter("") + return + + result = arg_converter("") + assert isinstance(result, dict) def test_arg_converter_replace_in_metadata(self): """Test arg_converter handles replace-in-metadata without assertions.""" - try: - result = arg_converter("--replace-in-metadata title foo bar") - except (ModuleNotFoundError, AttributeError, ImportError): - assert True + if importlib.util.find_spec("yt_dlp") is None: + with pytest.raises(ModuleNotFoundError): + arg_converter("--replace-in-metadata title foo bar") return + result = arg_converter("--replace-in-metadata title foo bar") + postprocessors = result.get("postprocessors", []) assert postprocessors, "Expected metadata parser postprocessor to be present" @@ -1584,11 +1585,10 @@ class TestLoadCookies: try: valid, jar = load_cookies(str(self.cookie_file)) - assert isinstance(valid, bool) - assert jar is not None or not valid + assert valid is False + assert jar is not None except ValueError: - # Expected for invalid cookie files - assert True + return class TestStrToDt: @@ -1596,20 +1596,23 @@ class TestStrToDt: def test_str_to_dt_basic(self): """Test basic string to datetime conversion.""" - try: - result = str_to_dt("2023-01-02 12:00:00 UTC") - assert isinstance(result, datetime) - except ModuleNotFoundError: - # Expected when dateparser is not available - assert True + if importlib.util.find_spec("dateparser") is None: + with pytest.raises(ModuleNotFoundError): + str_to_dt("2023-01-02 12:00:00 UTC") + return + + result = str_to_dt("2023-01-02 12:00:00 UTC") + assert isinstance(result, datetime) def test_str_to_dt_relative(self): """Test relative time string.""" - try: - result = str_to_dt("1 hour ago") - assert isinstance(result, datetime) - except (ModuleNotFoundError, ValueError): - assert True + if importlib.util.find_spec("dateparser") is None: + with pytest.raises(ModuleNotFoundError): + str_to_dt("1 hour ago") + return + + result = str_to_dt("1 hour ago") + assert isinstance(result, datetime) class TestInitClass: @@ -1633,37 +1636,6 @@ class TestInitClass: assert result.unused == "default" # Should use default -class TestLoadModules: - """Test the load_modules function.""" - - def setup_method(self): - """Set up test module structure.""" - self.temp_dir = str(make_test_temp_dir("load-modules")) - self.root_path = Path(self.temp_dir) - self.module_dir = self.root_path / "test_modules" - self.module_dir.mkdir() - - # Create test module files - (self.module_dir / "__init__.py").write_text("") - (self.module_dir / "test_module.py").write_text("# Test module") - - def teardown_method(self): - """Clean up after tests.""" - import shutil - - shutil.rmtree(self.temp_dir, ignore_errors=True) - - def test_load_modules_basic(self): - """Test basic module loading.""" - try: - load_modules(self.root_path, self.module_dir) - # Should not raise an exception - assert True - except Exception: - # Module loading might fail in test environment - assert True - - class TestGetChannelImages: """Test the get_channel_images function.""" @@ -1711,7 +1683,7 @@ class TestGetChannelImages: assert "icon" in result assert result["icon"] == "http://example.com/icon.jpg" - def test_get_channel_images_landscape_from_banner_uncropped(self): + def test_channel_images_landscape_banner(self): """Test extracting landscape from banner uncropped.""" from app.library.Utils import get_channel_images @@ -1859,23 +1831,27 @@ class TestArgConverterAdvanced: def test_arg_converter_with_removed_options(self): """Test arg_converter with removed options tracking.""" - try: - removed = [] - result = arg_converter("--quiet --skip-download", level=True, removed_options=removed) + if importlib.util.find_spec("yt_dlp") is None: + with pytest.raises(ModuleNotFoundError): + arg_converter("--quiet --skip-download", level=True, removed_options=[]) + return - assert isinstance(result, dict) - except (ModuleNotFoundError, AttributeError, ImportError): - assert True + removed = [] + result = arg_converter("--quiet --skip-download", level=True, removed_options=removed) + + assert isinstance(result, dict) + assert removed def test_arg_converter_dumps_enabled(self): """Test arg_converter with dumps flag enabled.""" - try: - result = arg_converter("--format best", dumps=True) + if importlib.util.find_spec("yt_dlp") is None: + with pytest.raises(ModuleNotFoundError): + arg_converter("--format best", dumps=True) + return - # With dumps=True, result should still be a dict with JSON-serializable content - assert isinstance(result, (dict, list)) - except (ModuleNotFoundError, AttributeError, ImportError): - assert True + result = arg_converter("--format best", dumps=True) + + assert isinstance(result, (dict, list)) class TestCreateCookiesFile: @@ -1910,7 +1886,7 @@ class TestCreateCookiesFile: @patch("app.library.config.Config") @patch("app.library.Utils.load_cookies") - def test_create_cookies_file_without_path(self, mock_load_cookies, mock_config): + def test_create_cookies_file_auto_path(self, mock_load_cookies, mock_config): """Test creating a cookies file without a specific path (auto temp file).""" from app.library.Utils import create_cookies_file @@ -1940,7 +1916,7 @@ class TestCreateCookiesFile: create_cookies_file("invalid_data", file=cookie_path) @patch("app.library.Utils.load_cookies") - def test_create_cookies_file_creates_parent_directory(self, mock_load_cookies): + def test_create_cookies_parent_dir(self, mock_load_cookies): """Test that create_cookies_file creates parent directories as needed.""" from app.library.Utils import create_cookies_file @@ -1955,7 +1931,7 @@ class TestCreateCookiesFile: assert cookie_path.parent == Path(self.test_path / "a" / "b" / "c") @patch("app.library.Utils.load_cookies") - def test_create_cookies_file_with_special_characters(self, mock_load_cookies): + def test_create_cookies_special_chars(self, mock_load_cookies): """Test create_cookies_file with special characters in cookie data.""" from app.library.Utils import create_cookies_file @@ -2099,7 +2075,7 @@ class TestRenameFile: assert subtitle_file.exists() assert conflicting_sidecar.exists() - def test_rename_preserves_sidecar_extensions(self, tmp_path: Path): + def test_rename_sidecar_extensions(self, tmp_path: Path): """Test that rename preserves complex sidecar extensions.""" # Create test files with complex extensions test_file = tmp_path / "video.mp4" diff --git a/ui/app/components/Simple.vue b/ui/app/components/Simple.vue index 3629ea2c..74290fa1 100644 --- a/ui/app/components/Simple.vue +++ b/ui/app/components/Simple.vue @@ -2,6 +2,12 @@
+
@@ -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/components/VideoPlayer.vue b/ui/app/components/VideoPlayer.vue index cfa67869..efb8d481 100644 --- a/ui/app/components/VideoPlayer.vue +++ b/ui/app/components/VideoPlayer.vue @@ -1,295 +1,377 @@ - - + + diff --git a/ui/app/composables/useConsoleSession.ts b/ui/app/composables/useConsoleSession.ts index 2b4ed6b0..930bcb55 100644 --- a/ui/app/composables/useConsoleSession.ts +++ b/ui/app/composables/useConsoleSession.ts @@ -3,6 +3,7 @@ import type { EventSourceMessage } from '@microsoft/fetch-event-source'; import { useStorage } from '@vueuse/core'; import { computed, ref } from 'vue'; +import type { TerminalSessionItem, TerminalSessionsResponse } from '~/types'; import { parse_api_error, parse_api_response, request, uri } from '~/utils'; type ConsoleSessionStatus = @@ -48,10 +49,25 @@ type CancelConsoleSessionResult = { message?: string; }; +type RecentSessionItem = { + sessionId: string; + command: string; + status: 'starting' | 'running' | 'completed'; + createdAt: number | null; + startedAt: number | null; + finishedAt: number | null; + expiresAt: number | null; + availableUntil: number | null; + exitCode: number | null; + lastSequence: number; +}; + const STORAGE_KEY = 'console_session_state'; +const HIDDEN_RECENT_SESSIONS_KEY = 'console_hidden_recent_sessions'; const MAX_TRANSCRIPT_CHUNKS = 1500; const MAX_TRANSCRIPT_CHARS = 120000; const RECONNECT_DELAY_MS = 1500; +const RECENT_SESSIONS_MAX = 30; const DEFAULT_STATE: ConsoleSessionState = { sessionId: null, @@ -160,6 +176,8 @@ const normalizePersistedState = ( }; const sessionState = useStorage(STORAGE_KEY, { ...DEFAULT_STATE }); +const hiddenRecentSessions = useStorage>(HIDDEN_RECENT_SESSIONS_KEY, []); +const recentSessions = ref>([]); const streamController = ref(null); const reconnectTimer = ref | null>(null); const connectionNonce = ref(0); @@ -176,6 +194,68 @@ const updateState = (patch: Partial): void => { ); }; +const normalizeHiddenRecentSessions = (): Array => { + return hiddenRecentSessions.value + .filter((value): value is string => typeof value === 'string') + .map((value) => value.trim()) + .filter((value) => Boolean(value)); +}; + +const setHiddenRecentSessions = (items: Array): void => { + hiddenRecentSessions.value = Array.from(new Set(items)); +}; + +const isRecentSessionHidden = (sessionId: string): boolean => { + return normalizeHiddenRecentSessions().includes(sessionId); +}; + +const sortRecentSessions = (items: Array): Array => { + return items.slice().sort((left, right) => { + const leftTime = left.finishedAt ?? left.startedAt ?? left.createdAt ?? 0; + const rightTime = right.finishedAt ?? right.startedAt ?? right.createdAt ?? 0; + return rightTime - leftTime; + }); +}; + +const trimRecentSessions = (items: Array): Array => { + return items.slice(0, RECENT_SESSIONS_MAX); +}; + +const setRecentSessions = (items: Array): void => { + recentSessions.value = trimRecentSessions( + sortRecentSessions(items.filter((item) => !isRecentSessionHidden(item.sessionId))), + ); +}; + +const upsertRecentSession = (item: RecentSessionItem): void => { + if (isRecentSessionHidden(item.sessionId)) { + return; + } + + setRecentSessions([ + item, + ...recentSessions.value.filter((entry) => entry.sessionId !== item.sessionId), + ]); +}; + +const hideRecentSession = (sessionId: string): void => { + const normalized = sessionId.trim(); + if (!normalized) { + return; + } + + setHiddenRecentSessions([...normalizeHiddenRecentSessions(), normalized]); + setRecentSessions(recentSessions.value.filter((item) => item.sessionId !== normalized)); +}; + +const clearRecentSessions = (): void => { + setHiddenRecentSessions([ + ...normalizeHiddenRecentSessions(), + ...recentSessions.value.map((item) => item.sessionId), + ]); + recentSessions.value = []; +}; + const appendTranscript = (chunk: string): void => { if (!chunk) { return; @@ -215,6 +295,7 @@ const clearTranscript = (): void => { const markExpired = (): void => { stopStream(); updateState({ status: 'expired', error: '' }); + syncCurrentSessionToRecentSessions(); }; const finishSession = ( @@ -227,6 +308,7 @@ const finishSession = ( exitCode, error: '', }); + syncCurrentSessionToRecentSessions(); }; const scheduleReconnect = (): void => { @@ -289,6 +371,97 @@ const parseResponseError = async (response: Response): Promise => { return response.statusText || 'Request failed.'; }; +const normalizeRecentSessionStatus = ( + status: string | null | undefined, +): RecentSessionItem['status'] => { + switch (normalizeStatus(status, 'finished')) { + case 'starting': + return 'starting'; + + case 'running': + case 'reconnecting': + return 'running'; + + default: + return 'completed'; + } +}; + +const recentSessionFromPayload = (payload: TerminalSessionItem): RecentSessionItem | null => { + if (typeof payload.session_id !== 'string' || !payload.session_id.trim()) { + return null; + } + + if (typeof payload.command !== 'string') { + return null; + } + + return { + sessionId: payload.session_id, + command: payload.command, + status: normalizeRecentSessionStatus(payload.status), + createdAt: typeof payload.created_at === 'number' ? payload.created_at : null, + startedAt: typeof payload.started_at === 'number' ? payload.started_at : null, + finishedAt: typeof payload.finished_at === 'number' ? payload.finished_at : null, + expiresAt: typeof payload.expires_at === 'number' ? payload.expires_at : null, + availableUntil: typeof payload.available_until === 'number' ? payload.available_until : null, + exitCode: typeof payload.exit_code === 'number' ? payload.exit_code : null, + lastSequence: typeof payload.last_sequence === 'number' ? payload.last_sequence : 0, + }; +}; + +const syncCurrentSessionToRecentSessions = (): void => { + if (!sessionState.value.sessionId || !sessionState.value.command.trim()) { + return; + } + + const existing = + recentSessions.value.find((item) => item.sessionId === sessionState.value.sessionId) ?? null; + const isFinished = ['finished', 'interrupted', 'error', 'expired'].includes( + sessionState.value.status, + ); + const lastSequence = + sessionState.value.lastEventId && /^\d+$/.test(sessionState.value.lastEventId) + ? Number(sessionState.value.lastEventId) + : (existing?.lastSequence ?? 0); + + upsertRecentSession({ + sessionId: sessionState.value.sessionId, + command: sessionState.value.command, + status: normalizeRecentSessionStatus(sessionState.value.status), + createdAt: existing?.createdAt ?? null, + startedAt: existing?.startedAt ?? null, + finishedAt: isFinished + ? (existing?.finishedAt ?? Date.now() / 1000) + : (existing?.finishedAt ?? null), + expiresAt: existing?.expiresAt ?? null, + availableUntil: existing?.availableUntil ?? null, + exitCode: sessionState.value.exitCode ?? existing?.exitCode ?? null, + lastSequence, + }); +}; + +const fetchRecentSessions = async (): Promise => { + try { + const response = await request('/api/system/terminal'); + if (!response.ok) { + return; + } + + const payload = await parse_api_response(response.json()); + const items = Array.isArray(payload?.items) + ? payload.items + .map((item) => recentSessionFromPayload(item)) + .filter((item): item is RecentSessionItem => item !== null) + : []; + + setRecentSessions(items); + syncCurrentSessionToRecentSessions(); + } catch { + return; + } +}; + const refreshSessionMetadata = async (): Promise => { if (!sessionState.value.sessionId) { return; @@ -303,10 +476,14 @@ const refreshSessionMetadata = async (): Promise => { } updateState(normalizeResponse(payload)); + syncCurrentSessionToRecentSessions(); } catch (error) { if (error instanceof ConsoleSessionExpiredError) { markExpired(); + return; } + + return; } }; @@ -400,6 +577,7 @@ const connectStream = async (): Promise => { onopen: async (response) => { if (response.ok) { updateState({ status: 'running', error: '' }); + syncCurrentSessionToRecentSessions(); return; } @@ -434,6 +612,7 @@ const connectStream = async (): Promise => { if (event.event === 'status') { updateState(normalizeResponse(payload as ConsoleSessionResponse)); + syncCurrentSessionToRecentSessions(); return; } @@ -501,6 +680,7 @@ const connectStream = async (): Promise => { const message = error instanceof Error ? error.message : String(error); appendTranscript(`Error: ${message}\n`); updateState({ status: 'error', error: message }); + syncCurrentSessionToRecentSessions(); } finally { if (streamController.value === controller) { streamController.value = null; @@ -517,12 +697,15 @@ const restoreSession = async (): Promise => { if (payload) { updateState(normalizeResponse(payload)); + syncCurrentSessionToRecentSessions(); } if (sessionState.value.sessionId && isActiveSessionStatus(sessionState.value.status)) { await connectStream(); } + await fetchRecentSessions(); + return; } @@ -531,28 +714,36 @@ const restoreSession = async (): Promise => { sessionState.value.command || sessionState.value.transcript.length > 0 ) { + await fetchRecentSessions(); return; } const active = await readSessionResponse('/api/system/terminal/active', { allowMissing: true }); if (!active) { + await fetchRecentSessions(); return; } updateState(normalizeResponse(active)); + syncCurrentSessionToRecentSessions(); if (sessionState.value.sessionId && isActiveSessionStatus(sessionState.value.status)) { await connectStream(); } + + await fetchRecentSessions(); } catch (error) { if (error instanceof ConsoleSessionExpiredError) { markExpired(); + await fetchRecentSessions(); return; } if (sessionState.value.sessionId && isActiveSessionStatus(sessionState.value.status)) { await connectStream(); } + + await fetchRecentSessions(); } }; @@ -594,16 +785,20 @@ const startSession = async ({ ]), error: '', }); + syncCurrentSessionToRecentSessions(); if (sessionState.value.sessionId && isActiveSessionStatus(sessionState.value.status)) { await connectStream(); } + await fetchRecentSessions(); + return Boolean(sessionState.value.sessionId); } catch (error) { const message = error instanceof Error ? error.message : String(error); appendTranscript(`Error: ${message}\n`); updateState({ status: 'error', error: message }); + syncCurrentSessionToRecentSessions(); return false; } }; @@ -623,6 +818,7 @@ const cancelSession = async (): Promise => { if (response.status === 404) { await refreshSessionMetadata(); + await fetchRecentSessions(); return { status: 'missing', message: 'Terminal session not found.' }; } @@ -631,6 +827,7 @@ const cancelSession = async (): Promise => { } updateState({ error: '' }); + await fetchRecentSessions(); return { status: 'cancelled' }; } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -642,6 +839,32 @@ const cancelSession = async (): Promise => { const resetState = (): void => { stopStream(); sessionState.value = { ...DEFAULT_STATE }; + recentSessions.value = []; + hiddenRecentSessions.value = []; +}; + +const replaySession = async (item: RecentSessionItem): Promise => { + stopStream(); + updateState({ + sessionId: item.sessionId, + command: item.command, + status: item.status === 'running' ? 'reconnecting' : 'finished', + lastEventId: null, + exitCode: item.exitCode, + transcript: [], + error: '', + }); + syncCurrentSessionToRecentSessions(); + + await connectStream(); + await fetchRecentSessions(); + + if (sessionState.value.status === 'expired') { + hideRecentSession(item.sessionId); + return false; + } + + return sessionState.value.status !== 'error'; }; const useConsoleSession = () => { @@ -649,7 +872,12 @@ const useConsoleSession = () => { state: sessionState, bufferedTranscript: computed(() => sessionState.value.transcript.slice()), isLoading: computed(() => isActiveSessionStatus(sessionState.value.status)), + recentSessions: computed(() => recentSessions.value.slice()), clearTranscript, + clearRecentSessions, + fetchRecentSessions, + hideRecentSession, + replaySession, restoreSession, startSession, cancelSession, diff --git a/ui/app/composables/useHistoryState.ts b/ui/app/composables/useHistoryState.ts index 82dae5cb..c10781a6 100644 --- a/ui/app/composables/useHistoryState.ts +++ b/ui/app/composables/useHistoryState.ts @@ -77,7 +77,7 @@ const buildQuery = ( return params.toString(); }; -const loadHistory = async (page: number = 1, options: HistoryLoadOptions = {}): Promise => { +const load = async (page: number = 1, options: HistoryLoadOptions = {}): Promise => { const { order = 'DESC', status, perPage = pageSize.value } = options; isLoading.value = true; @@ -104,12 +104,12 @@ const loadHistory = async (page: number = 1, options: HistoryLoadOptions = {}): } }; -const reloadHistory = async (options: HistoryLoadOptions = {}): Promise => { +const reload = async (options: HistoryLoadOptions = {}): Promise => { const targetPage = isLoaded.value ? pagination.value.page : 1; - await loadHistory(targetPage, options); + await load(targetPage, options); }; -const deleteHistoryItems = async ( +const remove = async ( options: { ids?: string[]; status?: string; @@ -155,7 +155,39 @@ const deleteHistoryItems = async ( } }; -const resetHistory = (): void => { +const rename = async (item: StoreItem, newName: string): Promise => { + const trimmedName = newName.trim(); + if (!trimmedName || trimmedName === item.filename?.split('/').pop()) { + return false; + } + + try { + const response = await request(`/api/history/${item._id}/rename`, { + method: 'POST', + body: JSON.stringify({ new_name: trimmedName }), + }); + + await ensureSuccess(response); + + const updated = (await response.json()) as StoreItem; + const index = items.value.findIndex((entry) => entry._id === updated._id); + + if (index !== -1) { + items.value = [...items.value.slice(0, index), updated, ...items.value.slice(index + 1)]; + } + + lastError.value = null; + return true; + } catch (error) { + handleError(error); + if (throwInstead.value) { + throw error; + } + return false; + } +}; + +const reset = (): void => { items.value = []; pagination.value = { page: 1, @@ -169,7 +201,7 @@ const resetHistory = (): void => { lastError.value = null; }; -const addHistoryItem = (item: StoreItem): void => { +const upsert = (item: StoreItem): void => { const existingIndex = items.value.findIndex((existing) => existing._id === item._id); if (existingIndex !== -1) { @@ -195,7 +227,7 @@ const addHistoryItem = (item: StoreItem): void => { pagination.value.has_next = pagination.value.page < pagination.value.total_pages; }; -const historyMoveHandler = ( +const moveHandler = ( shouldHandle: () => boolean = () => isLoaded.value, ): ((payload: WSEP['item_moved']) => void) => { return (payload: WSEP['item_moved']): void => { @@ -203,7 +235,7 @@ const historyMoveHandler = ( return; } - addHistoryItem(payload.data.item); + upsert(payload.data.item); }; }; @@ -215,11 +247,12 @@ export const useHistoryState = () => { isLoaded, lastError, pageSize, - loadHistory, - reloadHistory, - deleteHistoryItems, - resetHistory, - upsertHistoryItem: addHistoryItem, - historyMoveHandler, + load, + reload, + remove, + rename, + reset, + upsert, + moveHandler, }; }; diff --git a/ui/app/composables/useKeyboardShortcuts.ts b/ui/app/composables/useKeyboardShortcuts.ts index 2c5bb354..6093be72 100644 --- a/ui/app/composables/useKeyboardShortcuts.ts +++ b/ui/app/composables/useKeyboardShortcuts.ts @@ -5,21 +5,21 @@ import { ref } from 'vue'; import type { Ref } from 'vue'; import { - handlePlayPause, - handleRewind, - handleForward, - handleMute, - handleVolumeChange, - handlePlaybackSpeedChange, - handleFrameStep, - handleSeekToPercent, - handleSeekBackward, - handleSeekForward, - handleFullscreen, - handlePictureInPicture, - handleToggleCaptions, + playPause, + rewind, + forward, + mute, + changeVolume, + changeSpeed, + frameStep, + seekToPercent, + seekBackward, + seekForward, + fullscreen, + pictureInPicture, + toggleCaptions, shouldHandleKeyboardShortcut, - isModifierKey, + modifierKey, } from '~/utils/keyboard'; import type { KeyboardShortcutContext } from '~/types/video'; @@ -51,7 +51,7 @@ export const useKeyboardShortcuts = (options: UseKeyboardShortcutsOptions) => { } // Skip if modifier keys are pressed (except for shortcuts that need them) - if (isModifierKey(event) && !['f', 'p', '?'].includes(event.key.toLowerCase())) { + if (modifierKey(event) && !['f', 'p', '?'].includes(event.key.toLowerCase())) { return; } @@ -64,67 +64,67 @@ export const useKeyboardShortcuts = (options: UseKeyboardShortcutsOptions) => { case ' ': case 'k': event.preventDefault(); - handlePlayPause(ctx); + playPause(ctx); break; // Rewind 10 seconds (J key) case 'j': event.preventDefault(); - handleRewind(ctx, 10); + rewind(ctx, 10); break; // Forward 10 seconds (L key) case 'l': event.preventDefault(); - handleForward(ctx, 10); + forward(ctx, 10); break; // Seek backward 5 seconds (left arrow) case 'arrowleft': event.preventDefault(); - handleSeekBackward(ctx, 5); + seekBackward(ctx, 5); break; // Seek forward 5 seconds (right arrow) case 'arrowright': event.preventDefault(); - handleSeekForward(ctx, 5); + seekForward(ctx, 5); break; // Increase volume (up arrow) case 'arrowup': event.preventDefault(); - handleVolumeChange(ctx, 0.1); + changeVolume(ctx, 0.1); break; // Decrease volume (down arrow) case 'arrowdown': event.preventDefault(); - handleVolumeChange(ctx, -0.1); + changeVolume(ctx, -0.1); break; // Mute/Unmute case 'm': event.preventDefault(); - handleMute(ctx); + mute(ctx); break; // Toggle fullscreen case 'f': event.preventDefault(); - handleFullscreen(video); + fullscreen(video); break; // Picture-in-Picture case 'p': event.preventDefault(); - await handlePictureInPicture(video); + await pictureInPicture(video); break; // Toggle captions case 'c': event.preventDefault(); - handleToggleCaptions(video); + toggleCaptions(video); break; // Frame advance (period key) / Increase playback speed (> or ') @@ -132,9 +132,9 @@ export const useKeyboardShortcuts = (options: UseKeyboardShortcutsOptions) => { case "'": { event.preventDefault(); if ('.' === key) { - handleFrameStep(ctx, 'forward'); + frameStep(ctx, 'forward'); } else { - handlePlaybackSpeedChange(ctx, 0.25); + changeSpeed(ctx, 0.25); } break; } @@ -144,9 +144,9 @@ export const useKeyboardShortcuts = (options: UseKeyboardShortcutsOptions) => { case ';': { event.preventDefault(); if (',' === key) { - handleFrameStep(ctx, 'backward'); + frameStep(ctx, 'backward'); } else { - handlePlaybackSpeedChange(ctx, -0.25); + changeSpeed(ctx, -0.25); } break; } @@ -164,7 +164,7 @@ export const useKeyboardShortcuts = (options: UseKeyboardShortcutsOptions) => { case '9': { event.preventDefault(); const percent = parseInt(key) * 10; - handleSeekToPercent(ctx, percent); + seekToPercent(ctx, percent); break; } diff --git a/ui/app/composables/usePlayerMediaVolume.ts b/ui/app/composables/usePlayerMediaVolume.ts new file mode 100644 index 00000000..9856d332 --- /dev/null +++ b/ui/app/composables/usePlayerMediaVolume.ts @@ -0,0 +1,44 @@ +import { computed } from 'vue'; +import { useStorage } from '@vueuse/core'; + +function clampVolume(volume: number): number { + if (!Number.isFinite(volume)) return 1; + return Math.min(1, Math.max(0, volume)); +} + +export function usePlayerMediaVolume() { + const volume = useStorage('player_volume', 1); + const muted = useStorage('player_muted', false); + const effectiveVolume = computed(() => { + return muted.value ? 0 : clampVolume(volume.value); + }); + + function setVolume(nextVolume: number) { + const value = clampVolume(nextVolume); + volume.value = value; + muted.value = value <= 0; + } + + function changeVolume(delta: number) { + setVolume(volume.value + delta); + } + + function toggleMute() { + if (muted.value || effectiveVolume.value <= 0) { + volume.value = volume.value > 0 ? clampVolume(volume.value) : 1; + muted.value = false; + return; + } + + muted.value = true; + } + + return { + volume, + muted, + effectiveVolume, + setVolume, + changeVolume, + toggleMute, + }; +} diff --git a/ui/app/composables/usePlayerShortcutHelp.ts b/ui/app/composables/usePlayerShortcutHelp.ts new file mode 100644 index 00000000..7aee32c5 --- /dev/null +++ b/ui/app/composables/usePlayerShortcutHelp.ts @@ -0,0 +1,3 @@ +export function usePlayerShortcutHelp() { + return useState('player-shortcut-help', () => false); +} diff --git a/ui/app/composables/usePlayerShortcuts.ts b/ui/app/composables/usePlayerShortcuts.ts new file mode 100644 index 00000000..24300752 --- /dev/null +++ b/ui/app/composables/usePlayerShortcuts.ts @@ -0,0 +1,247 @@ +import { + getCurrentScope, + onScopeDispose, + ref, + watch, + type MaybeRefOrGetter, + type Ref, + toValue, +} from 'vue'; +import { + clampMediaTime, + clampMediaVolume, + hasModifierKey, + shouldHandleKeyboardShortcut, +} from '~/utils/keyboard'; + +type UsePlayerShortcutsOptions = { + enabled: MaybeRefOrGetter; + media: MaybeRefOrGetter; + video: MaybeRefOrGetter; + adjustVolume?: (delta: number) => void; + canToggleSubs: MaybeRefOrGetter; + helpOpen?: Ref; + toggleSubtitles: () => void; + toggleFullscreen: () => Promise | void; + toggleMute?: () => void; + closePlayer?: () => void; +}; + +export function usePlayerShortcuts(options: UsePlayerShortcutsOptions) { + const showHelp = options.helpOpen || ref(false); + + function togglePlayPause(media: HTMLMediaElement) { + if (media.paused) { + void media.play().catch(() => {}); + return; + } + + media.pause(); + } + + function stepFrame(media: HTMLMediaElement, direction: 'forward' | 'backward') { + if (!media.paused) { + media.pause(); + } + + const frameStep = direction === 'forward' ? 0.033 : -0.033; + clampMediaTime(media, media.currentTime + frameStep); + } + + function toggleNativeSubtitles(video: HTMLVideoElement) { + const tracks = Array.from(video.textTracks); + const subtitleTrack = tracks.find( + (track) => track.kind === 'subtitles' || track.kind === 'captions', + ); + if (!subtitleTrack) { + return; + } + + subtitleTrack.mode = subtitleTrack.mode === 'showing' ? 'hidden' : 'showing'; + } + + async function handleKeyDown(event: KeyboardEvent) { + if (!toValue(options.enabled) || !shouldHandleKeyboardShortcut(event)) { + return; + } + + const media = toValue(options.media); + if (!media) { + return; + } + + const key = event.key.toLowerCase(); + if (hasModifierKey(event) && !['f', '?', '/'].includes(key)) { + return; + } + + switch (key) { + case ' ': + case 'k': + event.preventDefault(); + event.stopPropagation(); + togglePlayPause(media); + break; + case 'j': + event.preventDefault(); + event.stopPropagation(); + clampMediaTime(media, media.currentTime - 10); + break; + case 'l': + event.preventDefault(); + event.stopPropagation(); + clampMediaTime(media, media.currentTime + 10); + break; + case 'arrowleft': + event.preventDefault(); + event.stopPropagation(); + clampMediaTime(media, media.currentTime - 5); + break; + case 'arrowright': + event.preventDefault(); + event.stopPropagation(); + clampMediaTime(media, media.currentTime + 5); + break; + case 'home': + event.preventDefault(); + event.stopPropagation(); + media.currentTime = 0; + break; + case 'end': + event.preventDefault(); + event.stopPropagation(); + if (Number.isFinite(media.duration)) { + media.currentTime = media.duration; + } + break; + case '0': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': { + event.preventDefault(); + event.stopPropagation(); + if (Number.isFinite(media.duration) && media.duration > 0) { + media.currentTime = (parseInt(key, 10) / 10) * media.duration; + } + break; + } + case 'arrowup': + event.preventDefault(); + event.stopPropagation(); + if (options.adjustVolume) { + options.adjustVolume(0.1); + break; + } + + media.volume = clampMediaVolume(media.volume + 0.1); + media.muted = false; + break; + case 'arrowdown': + event.preventDefault(); + event.stopPropagation(); + if (options.adjustVolume) { + options.adjustVolume(-0.1); + break; + } + + media.volume = clampMediaVolume(media.volume - 0.1); + if (media.volume <= 0) { + media.muted = true; + } + break; + case 'm': + event.preventDefault(); + event.stopPropagation(); + if (options.toggleMute) { + options.toggleMute(); + break; + } + + media.muted = !media.muted; + break; + case ';': + event.preventDefault(); + event.stopPropagation(); + media.playbackRate = Math.max(0.25, media.playbackRate - 0.25); + break; + case "'": + event.preventDefault(); + event.stopPropagation(); + media.playbackRate = Math.min(2, media.playbackRate + 0.25); + break; + case ',': + event.preventDefault(); + event.stopPropagation(); + stepFrame(media, 'backward'); + break; + case '.': + event.preventDefault(); + event.stopPropagation(); + stepFrame(media, 'forward'); + break; + case 'f': + event.preventDefault(); + event.stopPropagation(); + await options.toggleFullscreen(); + break; + case 'c': { + if (!toValue(options.canToggleSubs)) { + return; + } + + event.preventDefault(); + event.stopPropagation(); + const video = toValue(options.video); + if (video?.textTracks.length) { + toggleNativeSubtitles(video); + } + options.toggleSubtitles(); + break; + } + case '?': + case '/': + event.preventDefault(); + event.stopPropagation(); + showHelp.value = !showHelp.value; + break; + case 'escape': + event.preventDefault(); + event.stopPropagation(); + if (showHelp.value) { + showHelp.value = false; + break; + } + options.closePlayer?.(); + break; + default: + break; + } + } + + document.addEventListener('keydown', handleKeyDown, { capture: true }); + + watch( + () => toValue(options.enabled), + (enabled) => { + if (!enabled) { + showHelp.value = false; + } + }, + ); + + if (getCurrentScope()) { + onScopeDispose(() => { + document.removeEventListener('keydown', handleKeyDown, { capture: true }); + }); + } + + return { + showHelp, + }; +} diff --git a/ui/app/composables/usePlayerSubtitles.ts b/ui/app/composables/usePlayerSubtitles.ts new file mode 100644 index 00000000..355bf24a --- /dev/null +++ b/ui/app/composables/usePlayerSubtitles.ts @@ -0,0 +1,220 @@ +import { + computed, + getCurrentScope, + onScopeDispose, + ref, + watch, + type MaybeRefOrGetter, + toValue, +} from 'vue'; +import type { SubtitleManifestResponse, SubtitleTrack } from '~/types/subtitles'; +import { parse_api_error, request } from '~/utils'; + +type AssRendererInstance = { + destroy(): unknown; + show(): unknown; +}; + +type AssRendererConstructor = new ( + content: string, + video: HTMLVideoElement, + options: { container: HTMLElement; resampling: 'video_height' }, +) => AssRendererInstance; + +type UsePlayerSubtitlesOptions = { + manifestUrl: MaybeRefOrGetter; + isVideo: MaybeRefOrGetter; + canPlay: MaybeRefOrGetter; + shouldRender: MaybeRefOrGetter; + assLayoutVersion?: MaybeRefOrGetter; + video: MaybeRefOrGetter; + overlay: MaybeRefOrGetter; + fetchText?: (url: string) => Promise; + loadRenderer?: () => Promise; +}; + +async function defaultFetchSubtitleText(url: string): Promise { + const res = await request(url, { headers: { Accept: 'text/plain, text/vtt, text/x-ssa' } }); + if (!res.ok) { + throw new Error('Subtitle fetch failed'); + } + + return res.text(); +} + +async function defaultLoadAssRenderer(): Promise { + const mod = await import('assjs'); + return mod.default as AssRendererConstructor; +} + +export function usePlayerSubtitles(options: UsePlayerSubtitlesOptions) { + const fetchText = options.fetchText || defaultFetchSubtitleText; + const loadRenderer = options.loadRenderer || defaultLoadAssRenderer; + const tracks = ref([]); + const subtitleLoading = ref(false); + const subtitleLoadError = ref(''); + const subtitleEnabled = ref(true); + const selectedTrack = computed(() => tracks.value[0] || null); + const nativeSubtitleTrack = computed(() => { + const track = selectedTrack.value; + return subtitleEnabled.value && track?.renderer === 'native' ? track : null; + }); + const usesAssTrack = computed(() => selectedTrack.value?.renderer === 'assjs'); + const hasSubtitles = computed(() => tracks.value.length > 0); + + let assRenderer: AssRendererInstance | null = null; + let subtitleRequestId = 0; + let assRequestId = 0; + let cachedAssSubtitleUrl = ''; + let cachedAssSubtitleContent = ''; + + function destroyAssRenderer() { + assRenderer?.destroy(); + assRenderer = null; + } + + async function loadTracks() { + const manifestUrl = toValue(options.manifestUrl); + const isVideo = toValue(options.isVideo); + const canPlay = toValue(options.canPlay); + const requestId = ++subtitleRequestId; + + assRequestId += 1; + destroyAssRenderer(); + tracks.value = []; + subtitleLoadError.value = ''; + + if (!manifestUrl || !isVideo || !canPlay) { + subtitleLoading.value = false; + return; + } + + subtitleLoading.value = true; + + try { + const res = await request(manifestUrl); + const payload = (await res.json()) as SubtitleManifestResponse | { error?: string }; + if (!res.ok) { + throw new Error(await parse_api_error(payload)); + } + + if (requestId !== subtitleRequestId) { + return; + } + + tracks.value = (payload as SubtitleManifestResponse).subtitles || []; + if (tracks.value.length > 0) { + subtitleEnabled.value = true; + } + } catch { + if (requestId !== subtitleRequestId) { + return; + } + + subtitleLoadError.value = 'Failed to load subtitles for this video.'; + } finally { + if (requestId === subtitleRequestId) { + subtitleLoading.value = false; + } + } + } + + async function syncAssRenderer() { + const track = selectedTrack.value; + const shouldRender = toValue(options.shouldRender); + const video = toValue(options.video); + const overlay = toValue(options.overlay); + const requestId = ++assRequestId; + + destroyAssRenderer(); + + if ( + !track || + track.renderer !== 'assjs' || + !subtitleEnabled.value || + !shouldRender || + !video || + !overlay + ) { + return; + } + + try { + const subtitleContent = + cachedAssSubtitleUrl === track.url ? cachedAssSubtitleContent : await fetchText(track.url); + if (requestId !== assRequestId) { + return; + } + + if (cachedAssSubtitleUrl !== track.url) { + cachedAssSubtitleUrl = track.url; + cachedAssSubtitleContent = subtitleContent; + } + + const Ass = await loadRenderer(); + if (requestId !== assRequestId) { + return; + } + + assRenderer = new Ass(subtitleContent, video, { + container: overlay, + resampling: 'video_height', + }) as AssRendererInstance; + assRenderer.show(); + video.dispatchEvent(new Event('seeking')); + if (!video.paused) { + video.dispatchEvent(new Event('playing')); + } + subtitleLoadError.value = ''; + } catch { + if (requestId === assRequestId) { + subtitleLoadError.value = 'Failed to render ASS subtitles in the browser.'; + } + + destroyAssRenderer(); + } + } + + watch( + () => [toValue(options.manifestUrl), toValue(options.isVideo), toValue(options.canPlay)], + () => { + void loadTracks(); + }, + { immediate: true }, + ); + + watch( + () => [ + selectedTrack.value?.url || '', + selectedTrack.value?.renderer || '', + subtitleEnabled.value, + toValue(options.shouldRender), + toValue(options.assLayoutVersion) || 0, + toValue(options.video), + toValue(options.overlay), + ], + () => { + void syncAssRenderer(); + }, + { immediate: true }, + ); + + if (getCurrentScope()) { + onScopeDispose(() => { + assRequestId += 1; + destroyAssRenderer(); + }); + } + + return { + subtitleTracks: tracks, + subtitleLoading, + subtitleLoadError, + subtitleEnabled, + selectedSubtitleTrack: selectedTrack, + nativeSubtitleTrack, + usesAssSubtitleTrack: usesAssTrack, + hasSubtitles, + loadSelectedSubtitles: loadTracks, + }; +} diff --git a/ui/app/pages/browser/[...slug].vue b/ui/app/pages/browser/[...slug].vue index ba126a06..45afc5b8 100644 --- a/ui/app/pages/browser/[...slug].vue +++ b/ui/app/pages/browser/[...slug].vue @@ -1,5 +1,11 @@