Skip to content

Commit

Permalink
[BUGFIX] Have __preset__ override nested preset keys (#784)
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
jmbannon authored Oct 25, 2023
1 parent d810291 commit 852347d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 12 deletions.
6 changes: 4 additions & 2 deletions src/ytdl_sub/subscriptions/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,12 @@ def from_file_path(
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(
Expand Down
19 changes: 12 additions & 7 deletions src/ytdl_sub/subscriptions/subscription_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ def _indent_overrides_dict(self) -> Dict[str, str]:
}

@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 }``
Expand All @@ -81,15 +83,15 @@ def __init__(self, name, value, presets: List[str], indent_overrides: List[str])
_ = 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", [])

# Preset can be a single string
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()
)
Expand All @@ -115,15 +117,15 @@ def __init__(
)
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:
subscription_value_dict[self._subscription_value] = self.value

return {
self._leaf_name: {
"preset": self._presets,
"preset": self._presets + global_presets_to_apply,
"overrides": dict(
subscription_value_dict,
**self._indent_overrides_dict(),
Expand Down Expand Up @@ -196,9 +198,12 @@ def __init__(
)
)

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
14 changes: 11 additions & 3 deletions tests/unit/config/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,31 @@ 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": {
"nfo_name": "{uid}.nfo",
"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"},
Expand Down
17 changes: 17 additions & 0 deletions tests/unit/config/test_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ 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": {
"preset": "parent_preset_3",
"nfo_tags": {
"tags": {"key-4": "test_preset"},
},
"overrides": {"current_override": "test_preset"},
},
}

Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 852347d

Please sign in to comment.