diff --git a/docs/dev/rest-api/endpoints.md b/docs/dev/rest-api/endpoints.md index 4381318d..fe377943 100644 --- a/docs/dev/rest-api/endpoints.md +++ b/docs/dev/rest-api/endpoints.md @@ -439,8 +439,8 @@ Make a download job. It requires `service` and `title_id`. Every other field is | --- | --- | --- | --- | | `profile` | string | `null` | Credential/cookie profile. | | `quality` | int[] | `[]` | Target resolution(s); best available if empty. | -| `vcodec` | string/string[] | `null` | Video codec filter. | -| `acodec` | string/string[] | `null` | Audio codec filter. | +| `vcodec` | string/string[] | `[]` | Video codec filter. | +| `acodec` | string/string[] | `[]` | Audio codec filter. | | `vbitrate` / `abitrate` | int | `null` | Video / audio bitrate in kbps (positive). | | `vbitrate_range` / `abitrate_range` | string | `null` | `"MIN-MAX"` bitrate window. | | `range` | string[] | `["SDR"]` | Dynamic range(s). | @@ -460,6 +460,7 @@ Make a download job. It requires `service` and `title_id`. Every other field is | `sub_format` | string | `null` | Output subtitle format. | | `video_only` / `audio_only` / `subs_only` / `chapters_only` | boolean | `false` | Track-type restriction (at most one). | | `no_subs` / `no_audio` / `no_chapters` / `no_video` / `no_attachments` | boolean | `false` | Skip a track type. | +| `skip_subtitle_errors` | boolean | `false` | Skip a subtitle that fails to download and keep the other tracks. | | `audio_description` | boolean | `false` | Download audio description. | | `audio_description_only` | boolean | `false` | Download audio description and no standard audio. Not valid with `no_audio`, `video_only`, `subs_only` or `chapters_only`. | | `slow` | boolean/string | `null` | Randomized delay between downloads. | @@ -488,7 +489,7 @@ Make a download job. It requires `service` and `title_id`. Every other field is | `output_dir` | string | `null` | Output directory, relative to the server's downloads directory. The server rejects a path that resolves outside it with `400`. | | `no_cache` / `reset_cache` | boolean | `false` | Title cache controls. | -**Validation.** Invalid values return `400 INVALID_PARAMETERS`. `vcodec`, `acodec` and `sub_format` take the same spellings as the CLI options of the same name (`unshackle dl --help` prints each list). The API and the CLI read one list, so they cannot disagree. `range` must be one of SDR/HDR10/HDR10P/DV/HLG/HYBRID, and `HDR10+` is also valid. The bitrate, download worker, and download counts must be positive integers. You may set at most one of the `*_only` flags. You cannot combine `no_subs` with `subs_only`, or `no_audio` with `audio_only`. +**Validation.** Invalid values return `400 INVALID_PARAMETERS`. `vcodec`, `acodec` and `sub_format` take the same spellings as the CLI options of the same name (`unshackle dl --help` prints each list). The API and the CLI read one list, so they cannot disagree. `range` must be one of SDR/HDR10/HDR10P/DV/HLG/HYBRID, and `HDR10+` is also valid. The bitrate, download worker, and download counts must be positive integers. You may set at most one of the `*_only` flags. You cannot combine `no_subs` with `subs_only`, or `no_audio` with `audio_only`. The language fields (`lang`, `v_lang`, `a_lang`, `s_lang`, `forced_s_lang` and the `require_*` fields) must be arrays of strings: a single string such as `"en"` returns `400`. The server checks every other field with the type of its `dl` option. A `null` or an empty string gets the default from the table. **Service options.** A service's own CLI options travel in the same body. Send them in a nested `service_params` object, or as flat top-level keys. When the body has `service_params`, the server diff --git a/docs/dev/rest-api/quickstart.md b/docs/dev/rest-api/quickstart.md index f523c4a9..ad0df333 100644 --- a/docs/dev/rest-api/quickstart.md +++ b/docs/dev/rest-api/quickstart.md @@ -234,8 +234,8 @@ The request body accepts the full download parameter set. A few of the most usef | `title_id` | - | Title identifier (required) | | `profile` | `null` | Named credential/cookie profile to use | | `quality` | `[]` | Resolution(s), e.g. `[1080]`; empty means best available | -| `vcodec` | `null` | Video codec, e.g. `"H265"` | -| `acodec` | `null` | Audio codec, e.g. `"EC3"` | +| `vcodec` | `[]` | Video codec, e.g. `"H265"` | +| `acodec` | `[]` | Audio codec, e.g. `"EC3"` | | `range` | `["SDR"]` | Dynamic range: `SDR`, `HDR10`, `HDR10+`, `DV`, `HLG`, `HYBRID` | | `wanted` | `[]` | Episodes to fetch, e.g. `["S01E01"]`, or music tracks, e.g. `["1-5"]`. Empty means all | | `lang` | `["orig"]` | Video/audio language(s) | diff --git a/docs/guide/downloading.md b/docs/guide/downloading.md index 5ace9c3d..1db6f0ff 100644 --- a/docs/guide/downloading.md +++ b/docs/guide/downloading.md @@ -141,7 +141,7 @@ unshackle dl -v hevc,avc EXAMPLE 81234567 | `SDR` | Standard dynamic range (default) | | `HLG` | Hybrid Log-Gamma | | `HDR10` | HDR10 | -| `HDR10P` | HDR10+ | +| `HDR10P` | HDR10+ (`-r HDR10+` also works) | | `DV` | Dolby Vision | | `HYBRID` | Fetch both an HDR10/HDR10+ base and a DV track, then merge them | diff --git a/tests/core/test_click_types.py b/tests/core/test_click_types.py index e23e87a5..f57cf0f9 100644 --- a/tests/core/test_click_types.py +++ b/tests/core/test_click_types.py @@ -211,14 +211,6 @@ def test_no_duplicate_choices(param_type): assert len(lowered) == len(set(lowered)) -def test_api_codec_lists_come_from_the_param_types(): - from unshackle.core.api.handlers import VALID_ACODECS, VALID_SUB_FORMATS, VALID_VCODECS - - assert VALID_ACODECS == [c.upper() for c in AUDIO_CODEC_LIST.choices] - assert VALID_VCODECS == [c.upper() for c in VIDEO_CODEC_LIST.choices] - assert VALID_SUB_FORMATS == [c.upper() for c in SUBTITLE_CODEC.choices] - - @pytest.mark.parametrize("value", ["avc,,hevc", ["avc", "", "hevc"], " avc , hevc "]) def test_empty_video_codec_tokens_are_skipped(value): """A None in the list reaches track filtering and crashes it, so blanks must drop out.""" diff --git a/tests/core/test_hybrid_hdr10plus.py b/tests/core/test_hybrid_hdr10plus.py index 34f27540..aafcb633 100644 --- a/tests/core/test_hybrid_hdr10plus.py +++ b/tests/core/test_hybrid_hdr10plus.py @@ -591,7 +591,7 @@ def test_a_rest_job_gives_the_flag_to_the_service_ctx_and_to_dl( import click class FakeDl: - cli = click.Command("dl") + cli = dl_module.dl.cli def __init__(self, ctx: click.Context, **kwargs: object) -> None: self.ctx = ctx diff --git a/tests/lang_selection/test_required_langs.py b/tests/lang_selection/test_required_langs.py index fe74d943..7b51fbec 100644 --- a/tests/lang_selection/test_required_langs.py +++ b/tests/lang_selection/test_required_langs.py @@ -5,8 +5,7 @@ import inspect import pytest from unshackle.commands.dl import dl -from unshackle.core.api import download_manager -from unshackle.core.api.handlers import DEFAULT_DOWNLOAD_PARAMS +from unshackle.core.api.handlers import DEFAULT_DOWNLOAD_PARAMS, cast_dl_params from unshackle.core.utilities import missing_required_langs @@ -110,10 +109,9 @@ def test_api_accepts_the_require_flags(): [("require_audio", []), ("require_video", []), ("audio_description_only", False), ("forced_subs_only", False)], ) def test_api_reaches_dl_result(key, default): - # download_manager builds the dl.result kwargs by hand, so a missing key is a silent drop assert DEFAULT_DOWNLOAD_PARAMS[key] == default assert key in inspect.signature(dl.result).parameters - assert f'{key}=params.get("{key}", {default!r})' in inspect.getsource(download_manager) + assert cast_dl_params(dl.cli, {})[key] == default @pytest.mark.parametrize( diff --git a/tests/remote/unit/test_api_dl_options.py b/tests/remote/unit/test_api_dl_options.py new file mode 100644 index 00000000..d37f491a --- /dev/null +++ b/tests/remote/unit/test_api_dl_options.py @@ -0,0 +1,132 @@ +from __future__ import annotations + +import click +import pytest + +from unshackle.commands.dl import dl +from unshackle.core.api.handlers import ( + API_SHAPED_DL_PARAMS, + DEFAULT_DOWNLOAD_PARAMS, + cast_dl_params, + validate_download_parameters, +) +from unshackle.core.tracks import Video + +CLI_ONLY = { + "select_titles", + "list_", + "list_titles", + "import_file", + "cdm_name", + "remote", + "server", + "local_proxy", + "postscript", + "no_postscript", + "merge_video", + "real_video_bitrate", + "real_audio_bitrate", + "speed_limit", +} + + +def test_every_dl_option_is_an_api_param_or_cli_only(): + """A new dl option fails here until it is added to DEFAULT_DOWNLOAD_PARAMS or to CLI_ONLY.""" + api_names = set(cast_dl_params(dl.cli, {})) + cli_names = {p.name for p in dl.cli.params} + assert api_names - cli_names == set() + assert cli_names - api_names == CLI_ONLY + + +def test_api_defaults_cast_to_the_dl_defaults(): + """An API-shaped option keeps its own empty default, which perform_download normalizes.""" + ctx = click.Context(dl.cli) + expected = { + p.name: p.type_cast_value(ctx, p.get_default(ctx)) + for p in dl.cli.params + if p.name not in CLI_ONLY | API_SHAPED_DL_PARAMS + } + cast = cast_dl_params(dl.cli, {}) + assert {k: v for k, v in cast.items() if k not in API_SHAPED_DL_PARAMS} == expected + + +def test_cli_only_options_never_reach_dl(): + cast = cast_dl_params(dl.cli, {"postscript": ["rm -rf /"], "speed_limit": "1M", "list_": True}) + assert not CLI_ONLY & cast.keys() + + +@pytest.mark.parametrize( + "value,expected", + [ + ("HDR10+", [Video.Range.HDR10P]), + (["dv", "HDR10+"], [Video.Range.DV, Video.Range.HDR10P]), + ([], [Video.Range.SDR]), + (None, [Video.Range.SDR]), + ], +) +def test_range_takes_the_api_spellings(value, expected): + assert cast_dl_params(dl.cli, {"range": value})["range_"] == expected + + +def test_values_are_cast_by_the_dl_click_types(): + cast = cast_dl_params(dl.cli, {"quality": "1080p,720", "vcodec": "H265", "acodec": ["EC3"], "sample": True}) + assert cast["quality"] == [1080, 720] + assert cast["vcodec"] == [Video.Codec.HEVC] + assert [c.name for c in cast["acodec"]] == ["EC3"] + assert cast["sample"] is not None + + +@pytest.mark.parametrize("key", ["lang", "v_lang", "a_lang", "s_lang", "forced_s_lang"]) +@pytest.mark.parametrize("value", ["en", "en,fr", [1], {"x": "en"}]) +def test_api_rejects_a_non_array_language_value(key, value): + """A string would reach dl.result and split into one-character language codes.""" + assert validate_download_parameters({key: value}) == f"{key} must be an array of language strings" + + +@pytest.mark.parametrize( + "data,error", + [ + ({"tvdb_order": "nope"}, "Invalid tvdb_order"), + ({"tag": ["a", "b"]}, "Invalid tag: expected a single value"), + ({"proxy": {"host": "x"}}, "Invalid proxy: expected a single value"), + ], +) +def test_api_refuses_a_value_the_dl_option_cannot_take(data, error): + assert (validate_download_parameters(data) or "").startswith(error) + + +def test_no_dl_option_needs_a_parse_step_the_cast_skips(): + """type_cast_value runs no callback and no required check, so neither may appear on a dl option.""" + assert not [p.name for p in dl.cli.params if p.callback or p.required] + + +def test_null_takes_the_api_default(): + assert cast_dl_params(dl.cli, {key: None for key in DEFAULT_DOWNLOAD_PARAMS}) == cast_dl_params(dl.cli, {}) + + +@pytest.mark.parametrize("key", ["downloads", "workers", "channels", "latest_episodes"]) +def test_a_boolean_is_not_a_number(key): + with pytest.raises(click.BadParameter, match=f"Invalid {key}: expected a number"): + cast_dl_params(dl.cli, {key: True}) + + +@pytest.mark.parametrize("value", [[1], [None], {"a": 1}]) +def test_a_non_string_range_is_a_validation_error(value): + assert (validate_download_parameters({"range": value}) or "").startswith("Invalid range") + + +def test_the_worker_keeps_the_code_of_an_api_error(tmp_path, monkeypatch): + import json + + from unshackle.core.api import download_worker + from unshackle.core.api.errors import APIError, APIErrorCode + + def refuse(*args, **kwargs): + raise APIError(APIErrorCode.INVALID_PARAMETERS, "Invalid workers: expected a number, not a boolean") + + monkeypatch.setattr(download_worker, "perform_download", refuse) + monkeypatch.setattr(download_worker, "relay_prompts", lambda callback: None) + payload, result = tmp_path / "payload.json", tmp_path / "result.json" + payload.write_text(json.dumps({"job_id": "j", "service": "EXAMPLE", "title_id": "t", "parameters": {}})) + download_worker.main(["download_worker", str(payload), str(result)]) + assert json.loads(result.read_text())["error_code"] == "INVALID_PARAMETERS" diff --git a/tests/remote/unit/test_api_quality_param.py b/tests/remote/unit/test_api_quality_param.py index 83736751..0cf652ea 100644 --- a/tests/remote/unit/test_api_quality_param.py +++ b/tests/remote/unit/test_api_quality_param.py @@ -27,7 +27,7 @@ def test_the_api_accepts_the_quality_forms_the_cli_accepts(quality: Any) -> None def test_a_rest_job_converts_quality_and_passes_cdm_only(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: class FakeDl: - cli = click.Command("dl") + cli = dl_module.dl.cli def __init__(self, ctx: click.Context, **kwargs: object) -> None: self.ctx = ctx diff --git a/tests/remote/unit/test_download_gates_redaction.py b/tests/remote/unit/test_download_gates_redaction.py index c581a49c..06c6cd48 100644 --- a/tests/remote/unit/test_download_gates_redaction.py +++ b/tests/remote/unit/test_download_gates_redaction.py @@ -171,4 +171,4 @@ def test_range_validation_accepts_hdr10p_and_alias(): def test_range_validation_rejects_unknown_and_lists_hdr10p(): err = handlers.validate_download_parameters({"range": ["HDR99"]}) - assert err and "HDR10P" in err and "HDR99" in err + assert err and "hdr10p" in err.lower() and "HDR99" in err diff --git a/tests/remote/unit/test_download_manager.py b/tests/remote/unit/test_download_manager.py index 418880b8..b963d000 100644 --- a/tests/remote/unit/test_download_manager.py +++ b/tests/remote/unit/test_download_manager.py @@ -135,27 +135,14 @@ def test_job_status_values() -> None: } -def test_normalize_sub_format_preserves_original_sentinel() -> None: - """An API job asking for sub_format="original" must keep the "original" sentinel. - - dl.py treats "original" as "keep source format" (skip conversion). If it were normalized to - None (as to_enum(["original"]) yields, since there is no such enum member), dl.py would fall - into the default branch and re-encode TTML -> WebVTT, contradicting the CLI. - """ - from unshackle.core.api.download_manager import normalize_sub_format +def test_api_sub_format_keeps_the_original_sentinel() -> None: + """sub_format "original" keeps the source format, so the API cast must not turn it into a codec or None.""" + from unshackle.commands.dl import dl + from unshackle.core.api.handlers import cast_dl_params from unshackle.core.tracks import Subtitle - from unshackle.core.utils.click_types import SubtitleCodecChoice - # API path keeps the sentinel, case-insensitively. - assert normalize_sub_format("original") == "original" - assert normalize_sub_format("ORIGINAL") == "original" - - # The CLI produces the exact same sentinel value, so dl.py's `== "original"` branch handles both. - assert SubtitleCodecChoice(Subtitle.Codec).convert("original") == "original" - - # A real codec still maps to its enum member; unknowns collapse to None. - assert normalize_sub_format("srt") is Subtitle.Codec.SubRip - assert normalize_sub_format("nonsense") is None + assert cast_dl_params(dl.cli, {"sub_format": "ORIGINAL"})["sub_format"] == "original" + assert cast_dl_params(dl.cli, {"sub_format": "srt"})["sub_format"] is Subtitle.Codec.SubRip def test_worker_write_result_retries_denied_replace(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -218,7 +205,7 @@ def test_perform_download_puts_the_lang_selection_in_the_service_ctx( from unshackle.core.services import Services class FakeDl: - cli = click.Command("dl") + cli = dl_module.dl.cli def __init__(self, ctx: click.Context, **kwargs: object) -> None: raise _CapturedCtx(ctx) @@ -245,7 +232,7 @@ def test_perform_download_passes_latest_episodes_to_dl_result(tmp_path: Path, mo from unshackle.core.services import Services class FakeDl: - cli = click.Command("dl") + cli = dl_module.dl.cli def __init__(self, ctx: click.Context, **kwargs: object) -> None: pass @@ -268,6 +255,45 @@ def test_perform_download_passes_latest_episodes_to_dl_result(tmp_path: Path, mo assert exc.value.args[0]["latest_episodes"] == 3 +def test_perform_download_gives_dl_the_defaults_for_null_and_empty_values( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import click + + import unshackle.commands.dl as dl_module + from unshackle.core.api import download_manager, handlers + from unshackle.core.services import Services + + seen: dict[str, dict] = {} + + class FakeDl: + cli = dl_module.dl.cli + + def __init__(self, ctx: click.Context, **kwargs: object) -> None: + seen["init"] = kwargs + + def result(self, **kwargs: object) -> None: + seen["result"] = kwargs + raise RuntimeError("stop") + + class FakeService: + def __init__(self, ctx: click.Context) -> None: + seen["parent"] = dict(ctx.parent.params) + + monkeypatch.setattr(dl_module, "dl", FakeDl) + monkeypatch.setattr(Services, "get_path", staticmethod(lambda s: tmp_path)) + monkeypatch.setattr(Services, "load", staticmethod(lambda s: FakeService)) + monkeypatch.setattr(handlers, "load_full_cdm", lambda *a: None) + + params: dict[str, object] = {"quality": None, "vcodec": None, "acodec": None, "lang": None, "output_dir": ""} + with pytest.raises(RuntimeError, match="stop"): + download_manager.perform_download("job-1", "EXAMPLE", "t1", params) + + assert seen["init"]["output_dir"] is None + for view in (seen["parent"], seen["result"]): + assert view["quality"] == [] and view["vcodec"] == [] and view["acodec"] == [] and view["lang"] == ["orig"] + + def test_perform_download_casts_service_options(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: import click @@ -276,7 +302,7 @@ def test_perform_download_casts_service_options(tmp_path: Path, monkeypatch: pyt from unshackle.core.services import Services class FakeDl: - cli = click.Command("dl") + cli = dl_module.dl.cli def __init__(self, ctx: click.Context, **kwargs: object) -> None: pass diff --git a/tests/remote/unit/test_handler_proxy_policy.py b/tests/remote/unit/test_handler_proxy_policy.py index 3ca2f5ec..c96fc29c 100644 --- a/tests/remote/unit/test_handler_proxy_policy.py +++ b/tests/remote/unit/test_handler_proxy_policy.py @@ -305,7 +305,6 @@ class TestDownloadEnforcement: @pytest.mark.parametrize(("params", "expected"), [({}, []), ({"server_proxy": True}, None)]) def test_perform_download_maps_stamp_to_dl_providers(self, monkeypatch, tmp_path, params, expected): """An unstamped or denied job hands dl an empty provider list; a stamped one loads config.""" - import click from unshackle.commands import dl as dl_module from unshackle.core.api import download_manager as dm @@ -316,7 +315,7 @@ class TestDownloadEnforcement: pass class FakeDl: - cli = click.Group("dl") + cli = dl_module.dl.cli def __init__(self, **kwargs): seen.update(kwargs) diff --git a/tests/remote/unit/test_handlers_serialize.py b/tests/remote/unit/test_handlers_serialize.py index ffa01627..98af0677 100644 --- a/tests/remote/unit/test_handlers_serialize.py +++ b/tests/remote/unit/test_handlers_serialize.py @@ -291,7 +291,7 @@ def test_validate_download_params_accepts_defaults() -> None: "data, fragment", [ ({"vcodec": "WUT"}, "Invalid vcodec"), - ({"vcodec": 123}, "vcodec must be a string or list"), + ({"vcodec": 123}, "Invalid vcodec"), ({"acodec": "MP9"}, "Invalid acodec"), ({"sub_format": "doc"}, "Invalid sub_format"), ({"vbitrate": -1}, "vbitrate"), @@ -310,7 +310,7 @@ def test_validate_download_params_accepts_defaults() -> None: ({"no_audio": True, "audio_only": True}, "no_audio and audio_only"), ({"require_subs": "en"}, "require_subs must be an array"), ({"range": "UHD"}, "Invalid range"), - ({"range": ["SDR", "UHD"]}, "Invalid range value"), + ({"range": ["SDR", "UHD"]}, "Invalid range"), ], ) def test_validate_download_params_errors(data: dict, fragment) -> None: diff --git a/unshackle/core/api/download_manager.py b/unshackle/core/api/download_manager.py index ccba9957..d7e55bbb 100644 --- a/unshackle/core/api/download_manager.py +++ b/unshackle/core/api/download_manager.py @@ -19,7 +19,6 @@ from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, cast from unshackle.core.api.events import bus, publish_service_event from unshackle.core.api.sanitize import sanitize_log -from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, QUALITY_LIST, VIDEO_CODEC_LIST from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_all, redact_path, redact_text from unshackle.core.utils.service_params import cast_service_options, service_options @@ -334,28 +333,6 @@ def delete_job_history(job_id: str, allowed: Optional[set] = None, owner: Option return True -def to_enum(values: List[str], enum_cls: type[Enum]) -> List[Enum]: - """Map case-insensitive strings (by member name or value) to enum members, dropping unknowns.""" - lookup = {m.name.upper(): m for m in enum_cls} - lookup.update({str(m.value).upper(): m for m in enum_cls}) - return [lookup[v.upper()] for v in values if v.upper() in lookup] - - -def normalize_sub_format(sub_format_raw: str) -> Any: - """Normalise an API sub_format string to what dl.py expects. - - "original" is a sentinel meaning "keep source format" (dl.py skips conversion), so it must - pass through unchanged to match the CLI (SubtitleCodecChoice returns the same string). Any - other value maps to a Subtitle.Codec, or None when unrecognized. - """ - from unshackle.core.tracks import Subtitle - - if sub_format_raw.lower() == "original": - return "original" - mapped = to_enum([sub_format_raw], Subtitle.Codec) - return mapped[0] if mapped else None - - def client_sent_keys(params: Dict[str, Any]) -> set[str]: """Pop the server's ``client_keys`` stamp; a job without one counts every parameter as client-sent.""" return set(params.pop("client_keys", params)) @@ -370,11 +347,11 @@ def perform_download( ) -> List[str]: """Do the synchronous download work for a job. - `params` holds the API's string form of the `dl` options. This function normalizes `params` in - place before dl.result() sees it: vcodec and range names become enums, `slow` accepts "MIN-MAX", a two-item - list, or True (which means 60-120), and `wanted` accepts forms such as "S01E01", "S01-S03", - "s1e1", or "1x1" as well as the internal "SxE" form. Music takes track numbers in the same - field, such as "1-5" or "2x3" for disc 2 track 3. + `params` holds the API's string form of the `dl` options. The function first normalizes the API-shaped + options in place: `slow` accepts "MIN-MAX", a two-item list, or True (which means 60-120), and `wanted` + accepts forms such as "S01E01", "S01-S03", "s1e1", or "1x1" as well as the internal "SxE" form. Music takes track + numbers in the same field, such as "1-5" or "2x3" for disc 2 track 3. Then each dl click type casts its option + into ``ctx.params``, which feeds dl() and dl.result() as on the CLI. """ from contextlib import redirect_stderr, redirect_stdout @@ -387,7 +364,6 @@ def perform_download( from unshackle.core.api.errors import APIError, APIErrorCode from unshackle.core.config import config, load_yaml from unshackle.core.services import Services - from unshackle.core.tracks import Video from unshackle.core.utils.click_types import ContextData from unshackle.core.utils.collections import merge_dict @@ -411,42 +387,6 @@ def perform_download( client_keys = client_sent_keys(params) - vcodec_raw = params.get("vcodec") - if vcodec_raw: - params["vcodec"] = VIDEO_CODEC_LIST.convert(vcodec_raw) - else: - params["vcodec"] = [] - - if params.get("acodec"): - params["acodec"] = AUDIO_CODEC_LIST.convert(params["acodec"]) - - params["quality"] = QUALITY_LIST.convert(params["quality"]) if params.get("quality") else [] - - range_raw = params.get("range") - if range_raw: - if isinstance(range_raw, str): - range_raw = [range_raw] - if isinstance(range_raw, list) and range_raw and not isinstance(range_raw[0], Video.Range): - params["range"] = to_enum(range_raw, Video.Range) - else: - params["range"] = [Video.Range.SDR] - - sub_format_raw = params.get("sub_format") - if sub_format_raw and isinstance(sub_format_raw, str): - params["sub_format"] = normalize_sub_format(sub_format_raw) - - if params.get("export"): - params["export"] = bool(params["export"]) - - sample_raw = params.get("sample") - if sample_raw is not None: - from unshackle.core.utils.click_types import SAMPLE_RANGE - - try: - params["sample"] = SAMPLE_RANGE.convert(sample_raw, None, None) - except click.BadParameter as exc: - raise Exception(f"Invalid sample parameter: {exc}") - slow_raw = params.get("slow") if slow_raw is not None and not isinstance(slow_raw, tuple): if isinstance(slow_raw, bool): @@ -486,45 +426,17 @@ def perform_download( ctx = click.Context(dl_command.cli) ctx.invoked_subcommand = service - from unshackle.core.api.handlers import load_full_cdm, relay_label + from unshackle.core.api.handlers import cast_dl_params, load_full_cdm, relay_label cdm = load_full_cdm(service, params.get("profile"), params.get("cdm_type")) ctx.obj = ContextData(config=service_config, cdm=cdm, proxy_providers=[], profile=params.get("profile")) + try: + dl_params = cast_dl_params(dl_command.cli, params) + except click.BadParameter as exc: + raise APIError(APIErrorCode.INVALID_PARAMETERS, exc.message) ctx.params = { - "proxy": params.get("proxy"), - "no_proxy": params.get("no_proxy", False), - "no_proxy_download": params.get("no_proxy_download", False), - "proxy_download": params.get("proxy_download"), - "profile": params.get("profile"), + **dl_params, "cdm_name": params.get("cdm"), - "cdm_only": params.get("cdm_only"), - "all_drm": params.get("all_drm", False), - "repack": params.get("repack", False), - "tag": params.get("tag"), - "tmdb_id": params.get("tmdb_id"), - "imdb_id": params.get("imdb_id"), - "tvdb_id": params.get("tvdb_id"), - "tvdb_order": params.get("tvdb_order"), - "anilist_id": params.get("anilist_id"), - "enrich": params.get("enrich", False), - "daily": params.get("daily", False), - "output_dir": Path(params["output_dir"]) if params.get("output_dir") else None, - "no_cache": params.get("no_cache", False), - "reset_cache": params.get("reset_cache", False), - # Track-selection params read by Service.__init__ via ctx.parent.params - "quality": params.get("quality", []), - "vcodec": params.get("vcodec", []), - "range_": params.get("range", [Video.Range.SDR]), - "best_available": params.get("best_available", False), - "generate_dv": params.get("generate_dv", False), - # services read these here, not from dl.result(), to pick the manifests they fetch - "acodec": params.get("acodec") or [], - "lang": params.get("lang", ["orig"]), - "v_lang": params.get("v_lang", []), - "a_lang": params.get("a_lang", []), - "s_lang": params.get("s_lang", ["all"]), - "forced_subs": params.get("forced_subs", False), - "exact_lang": params.get("exact_lang", False), # dl skips the local service config and guards the proxy for a remote_services server "remote": bool(remote), "server": remote, @@ -540,23 +452,7 @@ def perform_download( param_name, ParameterSource.COMMANDLINE if source_key in client_keys else ParameterSource.DEFAULT ) - dl_instance = dl( - ctx=ctx, - no_proxy=params.get("no_proxy", False), - proxy_providers=None if params.get("server_proxy") is True else [], - profile=params.get("profile"), - proxy=params.get("proxy"), - repack=params.get("repack", False), - tag=params.get("tag"), - tmdb_id=params.get("tmdb_id"), - imdb_id=params.get("imdb_id"), - tvdb_id=params.get("tvdb_id"), - tvdb_order=params.get("tvdb_order"), - anilist_id=params.get("anilist_id"), - enrich=params.get("enrich", False), - daily=params.get("daily", False), - output_dir=Path(params["output_dir"]) if params.get("output_dir") else None, - ) + dl_instance = dl(ctx=ctx, proxy_providers=None if params.get("server_proxy") is True else [], **ctx.params) # Per-request credential ("user:pass"); feed it into the map get_credentials() reads so a # client can authenticate without anything being persisted to disk. Without a profile, # get_credentials() falls back to "default", so store it there too rather than dropping it @@ -618,68 +514,7 @@ def perform_download( with redirect_stdout(stdout_capture), redirect_stderr(stderr_capture): dl_instance.result( service=service_instance, - quality=params.get("quality", []), - vcodec=params.get("vcodec", []), - acodec=ctx.params["acodec"], - vbitrate=params.get("vbitrate"), - abitrate=params.get("abitrate"), - vbitrate_range=params.get("vbitrate_range"), - abitrate_range=params.get("abitrate_range"), - range_=params.get("range", ["SDR"]), - channels=params.get("channels"), - no_atmos=params.get("no_atmos", False), - select_titles=False, - wanted=params.get("wanted", []), - latest_episode=params.get("latest_episode", False), - latest_episodes=params.get("latest_episodes"), - lang=ctx.params["lang"], - v_lang=ctx.params["v_lang"], - a_lang=ctx.params["a_lang"], - s_lang=params.get("s_lang", ["all"]), - require_audio=params.get("require_audio", []), - require_video=params.get("require_video", []), - require_subs=params.get("require_subs", []), - forced_subs=ctx.params["forced_subs"], - forced_subs_only=params.get("forced_subs_only", False), - forced_s_lang=params.get("forced_s_lang", []), - exact_lang=params.get("exact_lang", False), - sub_format=params.get("sub_format"), - video_only=params.get("video_only", False), - audio_only=params.get("audio_only", False), - subs_only=params.get("subs_only", False), - chapters_only=params.get("chapters_only", False), - no_subs=params.get("no_subs", False), - skip_subtitle_errors=params.get("skip_subtitle_errors", False), - no_audio=params.get("no_audio", False), - no_chapters=params.get("no_chapters", False), - no_video=params.get("no_video", False), - no_attachments=params.get("no_attachments", False), - audio_description=params.get("audio_description", False), - audio_description_only=params.get("audio_description_only", False), - slow=params.get("slow", None), - list_=False, - list_titles=False, - skip_dl=params.get("skip_dl", False), - all_drm=params.get("all_drm", False), - export=params.get("export"), - cdm_only=params.get("cdm_only"), - no_proxy=params.get("no_proxy", False), - no_proxy_download=params.get("no_proxy_download", False), - proxy_download=params.get("proxy_download"), - no_folder=params.get("no_folder", False), - no_source=params.get("no_source", False), - no_mux=params.get("no_mux", False), - sample=params.get("sample"), - include_sample=params.get("include_sample", False), - workers=params.get("workers"), - adaptive_workers=params.get("adaptive_workers", False), - download_processes=params.get("download_processes", 1), - continue_downloads=params.get("continue_downloads", False), - downloads=params.get("downloads", 1), - worst=params.get("worst", False), - best_available=params.get("best_available", False), - generate_dv=params.get("generate_dv", False), - split_audio=params.get("split_audio"), + **{**ctx.params, "select_titles": False, "list_": False, "list_titles": False}, progress_sink=progress_callback, ) diff --git a/unshackle/core/api/errors.py b/unshackle/core/api/errors.py index 2950bbb6..35ea6a09 100644 --- a/unshackle/core/api/errors.py +++ b/unshackle/core/api/errors.py @@ -183,7 +183,7 @@ def categorize_exception( context: dict[str, Any] | None = None, ) -> APIError: """ - Categorize a generic exception into a structured APIError. + Categorize a generic exception into a structured APIError; an APIError is returned as it is. This function attempts to identify the type of error based on the exception type, message patterns, and optional context information. @@ -195,6 +195,8 @@ def categorize_exception( Returns: APIError with appropriate error code and details """ + if isinstance(exc, APIError): + return exc context = context or {} if isinstance(exc, SystemExit): exited = f"Service {context['service']} exited" if context.get("service") else "Service exited" @@ -324,9 +326,4 @@ def handle_api_exception( Returns: Structured JSON error response """ - if isinstance(exc, APIError): - api_error = exc - else: - api_error = categorize_exception(exc, context) - - return build_error_response(api_error, debug_mode, extra_debug_info) + return build_error_response(categorize_exception(exc, context), debug_mode, extra_debug_info) diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 167b4bfb..ae7dc7ef 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -55,7 +55,7 @@ from unshackle.core.tunnel import ( tunnel_proxy, ) from unshackle.core.utilities import declared_kwargs, kind_only_conflict, original_tag -from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, QUALITY_LIST, SUBTITLE_CODEC, VIDEO_CODEC_LIST +from unshackle.core.utils.click_types import AUDIO_CODEC_LIST, QUALITY_LIST, VIDEO_CODEC_LIST from unshackle.core.utils.collections import ci_get from unshackle.core.utils.hdr10plus import probed_tracks from unshackle.core.utils.redact import REDACTED, URL_USERINFO_RE, redact_all, redact_secrets, redact_text @@ -67,8 +67,8 @@ log = logging.getLogger("api") DEFAULT_DOWNLOAD_PARAMS = { "profile": None, "quality": [], - "vcodec": None, - "acodec": None, + "vcodec": [], + "acodec": [], "vbitrate": None, "abitrate": None, "vbitrate_range": None, @@ -96,6 +96,7 @@ DEFAULT_DOWNLOAD_PARAMS = { "subs_only": False, "chapters_only": False, "no_subs": False, + "skip_subtitle_errors": False, "no_audio": False, "no_chapters": False, "no_video": False, @@ -139,6 +140,33 @@ DEFAULT_DOWNLOAD_PARAMS = { "reset_cache": False, } +API_SHAPED_DL_PARAMS = frozenset({"slow", "wanted"}) + + +def cast_dl_params(cli: click.Command, params: Dict[str, Any]) -> Dict[str, Any]: + """Cast each ``DEFAULT_DOWNLOAD_PARAMS`` parameter in ``params`` by its dl click type, under its dl option name. + + A missing, ``None`` or empty-string parameter, and an empty ``range``, take the API default, so + ``output_dir: ""`` is not the working directory. An ``API_SHAPED_DL_PARAMS`` value stays as sent. A parameter + outside ``DEFAULT_DOWNLOAD_PARAMS`` never reaches dl, so a new CLI option is closed to API clients by default. + Raises ``click.BadParameter``. + """ + from unshackle.commands.dl import DL_OPTION_ALIASES + + kept: Dict[str, Any] = {} + to_cast: Dict[str, Any] = {} + for key, default in DEFAULT_DOWNLOAD_PARAMS.items(): + value = params.get(key) + if value is None or value == "" or (key == "range" and value == []): + value = default + (kept if value is None or key in API_SHAPED_DL_PARAMS else to_cast)[DL_OPTION_ALIASES.get(key, key)] = value + try: + return {**kept, **cast_service_options(cli.params, click.Context(cli), to_cast)} + except click.BadParameter as e: + name = (e.param.name if e.param else None) or "" + key = next((k for k, v in DL_OPTION_ALIASES.items() if v == name), name) + raise click.BadParameter(f"Invalid {key}: {e.message}") from e + # Keys that are part of the API transport envelope, not service.cli options. # Used by instantiate_service to avoid passing them as kwargs to a service. @@ -2164,11 +2192,6 @@ async def list_tracks_handler(data: Dict[str, Any], request: Optional[web.Reques await request_cache.cleanup() -VALID_VCODECS = [choice.upper() for choice in VIDEO_CODEC_LIST.choices] -VALID_ACODECS = [choice.upper() for choice in AUDIO_CODEC_LIST.choices] -VALID_SUB_FORMATS = [choice.upper() for choice in SUBTITLE_CODEC.choices] - - def resolve_vcodec(value: Any) -> Optional[list]: """Map a client's vcodec field to codec enums. @@ -2183,21 +2206,6 @@ def resolve_vcodec(value: Any) -> Optional[list]: raise APIError(APIErrorCode.INVALID_INPUT, f"Invalid vcodec: {e.format_message()}") -def check_codec(value: Any, allowed: List[str], name: str) -> Optional[str]: - """Validate a comma-string or list of codec tokens against `allowed` (case-insensitive).""" - if isinstance(value, str): - tokens = [v.strip() for v in value.split(",") if v.strip()] - elif isinstance(value, list): - tokens = [str(v).strip() for v in value if str(v).strip()] - else: - return f"{name} must be a string or list" - - invalid = [token for token in tokens if token.upper() not in allowed] - if invalid: - return f"Invalid {name}: {', '.join(invalid)}. Must be one of: {', '.join(allowed)}" - return None - - def validate_download_parameters(data: Dict[str, Any]) -> Optional[str]: """ Validate download parameters and return error message if invalid. @@ -2242,20 +2250,6 @@ def validate_download_parameters(data: Dict[str, Any]) -> Optional[str]: if data.get("all_drm") and data.get("cdm"): return "all_drm needs a CDM for each DRM system, so it cannot be used with cdm, which selects one device." - if "vcodec" in data and data["vcodec"]: - err = check_codec(data["vcodec"], VALID_VCODECS, "vcodec") - if err: - return err - - if "acodec" in data and data["acodec"]: - err = check_codec(data["acodec"], VALID_ACODECS, "acodec") - if err: - return err - - if "sub_format" in data and data["sub_format"]: - if str(data["sub_format"]).upper() not in VALID_SUB_FORMATS: - return f"Invalid sub_format: {data['sub_format']}. Must be one of: {', '.join(VALID_SUB_FORMATS)}" - if "vbitrate" in data and data["vbitrate"] is not None: if not isinstance(data["vbitrate"], int) or data["vbitrate"] <= 0: return "vbitrate must be a positive integer" @@ -2341,19 +2335,22 @@ def validate_download_parameters(data: Dict[str, Any]) -> Optional[str]: if isinstance(default, bool) and data.get(key) is not None and not isinstance(data[key], bool): return f"{key} must be a boolean" - for key in ("require_audio", "require_video", "require_subs"): + for key in ( + "lang", + "v_lang", + "a_lang", + "s_lang", + "forced_s_lang", + "require_audio", + "require_video", + "require_subs", + ): value = data.get(key) if value is not None and (not isinstance(value, list) or not all(isinstance(v, str) for v in value)): return f"{key} must be an array of language strings" ranges = data.get("range") or [] ranges = ranges if isinstance(ranges, list) else [ranges] - # "HDR10P" is the canonical range value ("+" is awkward in scripts); "HDR10+" stays valid. - valid_ranges = ["SDR", "HDR10", "HDR10P", "DV", "HLG", "HYBRID"] - accepted = {*valid_ranges, "HDR10+"} - for r in ranges: - if r.upper() not in accepted: - return f"Invalid range value: {r}. Must be one of: {', '.join(valid_ranges)}" if data.get("generate_dv"): # a request without a range runs with the serve: default range job_ranges = ranges or (config.serve or {}).get("range") or [] @@ -2361,6 +2358,13 @@ def validate_download_parameters(data: Dict[str, Any]) -> Optional[str]: if not any(str(r).upper() == "HYBRID" for r in job_ranges): return "generate_dv needs HYBRID in range." + from unshackle.commands.dl import dl + + try: + cast_dl_params(dl.cli, data) + except click.BadParameter as e: + return e.message + return None diff --git a/unshackle/core/api/routes.py b/unshackle/core/api/routes.py index d62ee827..29f7000a 100644 --- a/unshackle/core/api/routes.py +++ b/unshackle/core/api/routes.py @@ -926,14 +926,14 @@ async def download(request: web.Request) -> web.Response: - type: array items: type: string - description: Video codec(s) to download (e.g., "HEVC" or ["AVC", "HEVC"]) - accepts AVC, H.264, H264, HEVC, H.265, H265, VC1, VC-1, VP8, VP9, AV1, LCEVC (default - None) + description: Video codec(s) to download (e.g., "HEVC" or ["AVC", "HEVC"]) - accepts AVC, H.264, H264, HEVC, H.265, H265, VC1, VC-1, VP8, VP9, AV1, LCEVC (default - []) acodec: oneOf: - type: string - type: array items: type: string - description: Audio codec(s) to download (e.g., "AAC" or ["AAC", "EC3"]) - accepts AAC, AC3, DD, EC3, DD+, EAC3, DDP, AC4, AC-4, OPUS, OGG, VORB, VORBIS, DTS, DTSX, DTS-X, DTSHD, DTS-HD, MPEGH, MPEG-H, ALAC, FLAC, MP3 (default - None) + description: Audio codec(s) to download (e.g., "AAC" or ["AAC", "EC3"]) - accepts AAC, AC3, DD, EC3, DD+, EAC3, DDP, AC4, AC-4, OPUS, OGG, VORB, VORBIS, DTS, DTSX, DTS-X, DTSHD, DTS-HD, MPEGH, MPEG-H, ALAC, FLAC, MP3 (default - []) vbitrate: type: integer description: Video bitrate in kbps (default - None) @@ -1032,6 +1032,9 @@ async def download(request: web.Request) -> web.Response: no_subs: type: boolean description: Do not download subtitle tracks (default - false) + skip_subtitle_errors: + type: boolean + description: Skip a subtitle that fails to download and keep the other tracks (default - false) no_audio: type: boolean description: Do not download audio tracks (default - false) diff --git a/unshackle/core/utils/click_types.py b/unshackle/core/utils/click_types.py index 5e4797f8..31b618a6 100644 --- a/unshackle/core/utils/click_types.py +++ b/unshackle/core/utils/click_types.py @@ -1,5 +1,6 @@ import re from datetime import date, timedelta +from enum import Enum from typing import Any, Optional, Sequence, Union import click @@ -434,7 +435,7 @@ class AudioCodecList(click.ParamType): class MultipleChoice(click.Choice): """ The multiple choice type takes several values. Each value must be in a fixed - set of permitted values. + set of permitted values. An Enum choice also matches its value, so HDR10+ selects HDR10P. It internally uses and extends click.Choice. """ @@ -458,7 +459,11 @@ class MultipleChoice(click.Choice): chosen_values: list[Any] = [] for value in values: - chosen_values.append(super().convert(value, param, ctx)) + member = next( + (c for c in self.choices if isinstance(c, Enum) and str(c.value).casefold() == str(value).casefold()), + None, + ) + chosen_values.append(member if member is not None else super().convert(value, param, ctx)) return chosen_values diff --git a/unshackle/core/utils/service_params.py b/unshackle/core/utils/service_params.py index 7e598185..65bcbcc2 100644 --- a/unshackle/core/utils/service_params.py +++ b/unshackle/core/utils/service_params.py @@ -23,8 +23,9 @@ def service_options( def cast_service_options(params: Iterable[click.Parameter], ctx: Any, options: Dict[str, Any]) -> Dict[str, Any]: """``options`` with each value cast by the click parameter of the same name; raises ``click.BadParameter``. - A name that no parameter declares keeps its value. A JSON object, or an array for a single-value option, - is refused before the cast, since click would stringify it. + A name that no parameter declares keeps its value. A JSON object, an array for a single-value text option + (click would stringify it) and a boolean for a number are refused before the cast. ``convert()`` only promises + ``BadParameter`` for strings, so any other error of a JSON value becomes one too. """ by_name = {p.name: p for p in params if p.name} cast: Dict[str, Any] = {} @@ -33,10 +34,17 @@ def cast_service_options(params: Iterable[click.Parameter], ctx: Any, options: D if param is None: cast[k] = v continue - if isinstance(v, dict) or (isinstance(v, list) and not param.multiple and param.nargs == 1): + if isinstance(v, dict) or ( + isinstance(v, list) + and not param.multiple + and param.nargs == 1 + and isinstance(param.type, click.types.StringParamType) + ): raise click.BadParameter("expected a single value, not a JSON object or array", ctx=ctx, param=param) + if isinstance(v, bool) and isinstance(param.type, (click.types.IntParamType, click.types.FloatParamType)): + raise click.BadParameter("expected a number, not a boolean", ctx=ctx, param=param) try: cast[k] = param.type_cast_value(ctx, v) - except (TypeError, ValueError, AttributeError) as e: # convert() only promises BadParameter for strings + except (TypeError, ValueError, AttributeError, OverflowError) as e: raise click.BadParameter(f"not a valid {param.type.name} value", ctx=ctx, param=param) from e return cast