From 852347dc7e9b6657cb5f3309708406babcea5740 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 25 Oct 2023 11:35:52 -0700 Subject: [PATCH] [BUGFIX] Have `__preset__` override nested preset keys (#784) With the new preset nesting support, `__preset__` would have lower precedence which isn't ideal since it's meant to serve as a 'apply to all presets in this file'. Now the order of priority is: `inherited presets (top-to-bottom) -> preset itself -> nested subscription presets -> __preset__ -> subscription values` --- src/ytdl_sub/subscriptions/subscription.py | 6 ++++-- .../subscriptions/subscription_validators.py | 19 ++++++++++++------- tests/unit/config/conftest.py | 14 +++++++++++--- tests/unit/config/test_subscription.py | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/ytdl_sub/subscriptions/subscription.py b/src/ytdl_sub/subscriptions/subscription.py index 8cf66dcb..91989411 100644 --- a/src/ytdl_sub/subscriptions/subscription.py +++ b/src/ytdl_sub/subscriptions/subscription.py @@ -147,10 +147,12 @@ class Subscription(SubscriptionDownload): name="", value=subscriptions_dict, config=config, - presets=[FILE_PRESET_APPLY_KEY] if has_file_preset else [], + presets=[], indent_overrides=[], subscription_value=file_subscription_value, - ).subscription_dicts() + ).subscription_dicts( + global_presets_to_apply=[FILE_PRESET_APPLY_KEY] if has_file_preset else [] + ) for subscription_key, subscription_object in subscriptions_dicts.items(): subscriptions.append( diff --git a/src/ytdl_sub/subscriptions/subscription_validators.py b/src/ytdl_sub/subscriptions/subscription_validators.py index 2b15f065..1d3686a9 100644 --- a/src/ytdl_sub/subscriptions/subscription_validators.py +++ b/src/ytdl_sub/subscriptions/subscription_validators.py @@ -66,8 +66,10 @@ class SubscriptionOutput(Validator, ABC): } @abstractmethod - def subscription_dicts(self) -> Dict[str, Dict]: + def subscription_dicts(self, global_presets_to_apply: List[str]) -> Dict[str, Dict]: """ + Parameters + Returns ------- Subscriptions in the form of ``{ subscription_name: preset_dict }`` @@ -81,7 +83,7 @@ class SubscriptionPresetDictValidator(SubscriptionOutput, DictValidator): _ = self._validate_key_if_present(key="preset", validator=StringListValidator, default=[]) _ = self._validate_key_if_present(key="overrides", validator=Overrides, default={}) - def subscription_dicts(self) -> Dict[str, Dict]: + def subscription_dicts(self, global_presets_to_apply: List[str]) -> Dict[str, Dict]: output_dict = copy.deepcopy(self._dict) parent_presets = output_dict.get("preset", []) @@ -89,7 +91,7 @@ class SubscriptionPresetDictValidator(SubscriptionOutput, DictValidator): if isinstance(parent_presets, str): parent_presets = [parent_presets] - output_dict["preset"] = parent_presets + self._presets + output_dict["preset"] = parent_presets + self._presets + global_presets_to_apply output_dict["overrides"] = dict( output_dict.get("overrides", {}), **self._indent_overrides_dict() ) @@ -115,7 +117,7 @@ class SubscriptionValueValidator(SubscriptionOutput, StringValidator): ) self._subscription_value: Optional[str] = subscription_value - def subscription_dicts(self) -> Dict[str, Dict]: + def subscription_dicts(self, global_presets_to_apply: List[str]) -> Dict[str, Dict]: subscription_value_dict: Dict[str, str] = {"subscription_value": self.value} # TODO: Eventually delete in favor of {subscription_value} if self._subscription_value: @@ -123,7 +125,7 @@ class SubscriptionValueValidator(SubscriptionOutput, StringValidator): return { self._leaf_name: { - "preset": self._presets, + "preset": self._presets + global_presets_to_apply, "overrides": dict( subscription_value_dict, **self._indent_overrides_dict(), @@ -196,9 +198,12 @@ class SubscriptionValidator(SubscriptionOutput): ) ) - def subscription_dicts(self) -> Dict[str, Dict]: + def subscription_dicts(self, global_presets_to_apply: List[str]) -> Dict[str, Dict]: subscription_dicts: Dict[str, Dict] = {} for child in self._children: - subscription_dicts = dict(subscription_dicts, **child.subscription_dicts()) + subscription_dicts = dict( + subscription_dicts, + **child.subscription_dicts(global_presets_to_apply=global_presets_to_apply), + ) return subscription_dicts diff --git a/tests/unit/config/conftest.py b/tests/unit/config/conftest.py index 98c40e15..f3d3195d 100644 --- a/tests/unit/config/conftest.py +++ b/tests/unit/config/conftest.py @@ -12,7 +12,10 @@ def config_file() -> ConfigFile: value={ "configuration": {"working_directory": "."}, "presets": { - "parent_preset_0": {"nfo_tags": {"tags": {"key-1": "preset_0"}}}, + "parent_preset_0": { + "nfo_tags": {"tags": {"key-1": "preset_0"}}, + "overrides": {"current_override": "parent_preset_0"}, + }, "parent_preset_1": { "preset": "parent_preset_0", "nfo_tags": { @@ -20,15 +23,20 @@ def config_file() -> ConfigFile: "nfo_root": "root", "tags": {"key-2": "preset_1"}, }, + "overrides": {"current_override": "parent_preset_1"}, }, "parent_preset_2": { "nfo_tags": { "nfo_name": "{uid}.nfo", "nfo_root": "root", "tags": {"key-2": "preset_2", "key-3": "preset_2"}, - } + }, + "overrides": {"current_override": "parent_preset_2"}, + }, + "parent_preset_3": { + "preset": ["parent_preset_1", "parent_preset_2"], + "overrides": {"current_override": "parent_preset_3"}, }, - "parent_preset_3": {"preset": ["parent_preset_1", "parent_preset_2"]}, "preset_self_loop": {"preset": "preset_self_loop"}, "preset_loop_0": {"preset": "preset_loop_1"}, "preset_loop_1": {"preset": "preset_loop_0"}, diff --git a/tests/unit/config/test_subscription.py b/tests/unit/config/test_subscription.py index 849c7823..b64272f0 100644 --- a/tests/unit/config/test_subscription.py +++ b/tests/unit/config/test_subscription.py @@ -47,6 +47,7 @@ def preset_with_file_preset(youtube_video: Dict, output_options: Dict): "test_config_subscription_value": "original", "subscription_indent_1": "original_1", "subscription_indent_2": "original_2", + "current_override": "__preset__", }, }, "test_preset": { @@ -54,6 +55,7 @@ def preset_with_file_preset(youtube_video: Dict, output_options: Dict): "nfo_tags": { "tags": {"key-4": "test_preset"}, }, + "overrides": {"current_override": "test_preset"}, }, } @@ -137,6 +139,8 @@ def test_subscription_file_preset_applies(config_file: ConfigFile, preset_with_f # Test __preset__ worked correctly preset_sub = subs[0] + assert preset_sub.name == "test_preset" + nfo_options: NfoTagsOptions = preset_sub.plugins.get(NfoTagsOptions) tags_string_dict = { key: formatter[0].format_string for key, formatter in nfo_options.tags.string_tags.items() @@ -149,6 +153,10 @@ def test_subscription_file_preset_applies(config_file: ConfigFile, preset_with_f "key-4": "test_preset", } + overrides = preset_sub.overrides.dict_with_format_strings + # preset overrides take precedence over __preset__ + assert overrides.get("current_override") == "test_preset" + def test_subscription_file_value_applies( config_file: ConfigFile, preset_with_subscription_file_value: Dict @@ -165,6 +173,7 @@ def test_subscription_file_value_applies( assert overrides.get("test_file_subscription_value") == "is_overwritten" assert overrides.get("test_file_subscription_value") assert overrides.get("subscription_value") == "is_overwritten" + assert overrides.get("current_override") == "__preset__" # ensure __preset__ takes precedence def test_subscription_file_value_applies_sub_file_takes_precedence( @@ -183,6 +192,7 @@ def test_subscription_file_value_applies_sub_file_takes_precedence( assert value_sub.get("test_config_subscription_value") == "original" assert value_sub.get("subscription_name") == "test_value" assert value_sub.get("subscription_value") == "is_overwritten" + assert value_sub.get("current_override") == "__preset__" # ensure __preset__ takes precedence def test_subscription_file_value_applies_from_config( @@ -200,6 +210,7 @@ def test_subscription_file_value_applies_from_config( assert value_sub.get("test_config_subscription_value") == "is_overwritten" assert value_sub.get("subscription_name") == "test_value" assert value_sub.get("subscription_value") == "is_overwritten" + assert value_sub.get("current_override") == "__preset__" # ensure __preset__ takes precedence def test_subscription_file_value_applies_from_config_and_nested( @@ -219,10 +230,12 @@ def test_subscription_file_value_applies_from_config_and_nested( assert sub_1.get("test_config_subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_name") == "test_1" assert sub_1.get("subscription_value") == "is_1_overwritten" + assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_name") == "test_2_1" assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" + assert sub_2_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence def test_subscription_file_value_applies_from_config_and_nested_and_indent_variables( @@ -252,12 +265,14 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia assert sub_1.get("subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_indent_1") == "INDENT_1" assert sub_1.get("subscription_indent_2") == "INDENT_2" + assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_name") == "test_2_1" assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_indent_1") == "INDENT_1" assert sub_2_1.get("subscription_indent_2") == "original_2" + assert sub_2_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence def test_subscription_file_value_applies_from_config_and_nested_and_indent_variables_same_line( @@ -288,12 +303,14 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia assert sub_1.get("subscription_indent_1") == "INDENT_1" assert sub_1.get("subscription_indent_2") == "INDENT_2" assert sub_1.get("subscription_indent_3") == "INDENT_3" + assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_name") == "test_2_1" assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_indent_1") == "INDENT_1" assert sub_2_1.get("subscription_indent_2") == "original_2" + assert sub_2_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence assert "subscription_indent_3" not in sub_2_1