Skip to content

Commit

Permalink
[BUGFIX] Fix variable collision when disabling plugins (#1072)
Browse files Browse the repository at this point in the history
Fixes #1071
  • Loading branch information
jmbannon authored Sep 30, 2024
1 parent 1b57d79 commit f2ec26d
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 31 deletions.
1 change: 0 additions & 1 deletion src/ytdl_sub/config/plugin/plugin_operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


class PluginOperation(Enum):
ANY = -2
DOWNLOADER = -1
MODIFY_ENTRY_METADATA = 0
MODIFY_ENTRY = 1
Expand Down
2 changes: 0 additions & 2 deletions src/ytdl_sub/config/validators/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ def modified_variables(self) -> Dict[PluginOperation, Set[str]]:

def added_variables(
self,
resolved_variables: Set[str],
unresolved_variables: Set[str],
plugin_op: PluginOperation,
) -> Dict[PluginOperation, Set[str]]:
"""
If the plugin adds source variables, list them here.
Expand Down
4 changes: 0 additions & 4 deletions src/ytdl_sub/config/validators/variable_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ def _get_added_and_modified_variables(
modified_variables: Set[str] = set()

for plugin_added_variables in plugin_options.added_variables(
resolved_variables=set(),
unresolved_variables=set(),
plugin_op=PluginOperation.ANY,
).values():
added_variables |= set(plugin_added_variables)

Expand Down Expand Up @@ -167,9 +165,7 @@ def _add_variables(self, plugin_op: PluginOperation, options: OptionsValidator)
Add dummy variables for script validation
"""
added_variables = options.added_variables(
resolved_variables=self.resolved_variables,
unresolved_variables=self.unresolved_variables,
plugin_op=plugin_op,
).get(plugin_op, set())
modified_variables = options.modified_variables().get(plugin_op, set())

Expand Down
21 changes: 9 additions & 12 deletions src/ytdl_sub/downloaders/url/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,24 +320,21 @@ def variables(self) -> DictFormatterValidator:

def added_variables(
self,
resolved_variables: Set[str],
unresolved_variables: Set[str],
plugin_op: PluginOperation,
) -> Dict[PluginOperation, Set[str]]:
"""
Returns
-------
List of variables added. The first collection url always contains all the variables.
"""
if plugin_op != PluginOperation.ANY:
for url in self._urls.list:
for variable_name, definition in url.variables.dict_with_format_strings.items():
used_variables = set(var.name for var in parse(definition).variables)
if unresolved := used_variables & unresolved_variables:
raise self._validation_exception(
f"variable {variable_name} cannot use the variables "
f"{', '.join(sorted(list(unresolved)))} because it depends on other"
" variables that are computed later in execution"
)
for url in self._urls.list:
for variable_name, definition in url.variables.dict_with_format_strings.items():
used_variables = set(var.name for var in parse(definition).variables)
if unresolved := used_variables & unresolved_variables:
raise self._validation_exception(
f"variable {variable_name} cannot use the variables "
f"{', '.join(sorted(list(unresolved)))} because it depends on other"
" variables that are computed later in execution"
)

return {PluginOperation.DOWNLOADER: set(self._urls.list[0].variables.keys)}
2 changes: 0 additions & 2 deletions src/ytdl_sub/plugins/chapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,7 @@ def force_key_frames(self) -> bool:

def added_variables(
self,
resolved_variables: Set[str],
unresolved_variables: Set[str],
plugin_op: PluginOperation,
) -> Dict[PluginOperation, Set[str]]:
return {PluginOperation.MODIFY_ENTRY: {ytdl_sub_chapters_from_comments.variable_name}}

Expand Down
2 changes: 0 additions & 2 deletions src/ytdl_sub/plugins/split_by_chapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,7 @@ def __init__(self, name, value):

def added_variables(
self,
resolved_variables: Set[str],
unresolved_variables: Set[str],
plugin_op: PluginOperation,
) -> Dict[PluginOperation, Set[str]]:
return {
PluginOperation.MODIFY_ENTRY: {
Expand Down
2 changes: 0 additions & 2 deletions src/ytdl_sub/plugins/subtitles.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ def allow_auto_generated_subtitles(self) -> Optional[bool]:

def added_variables(
self,
resolved_variables: Set[str],
unresolved_variables: Set[str],
plugin_op: PluginOperation,
) -> Dict[PluginOperation, Set[str]]:
"""
Returns
Expand Down
19 changes: 14 additions & 5 deletions src/ytdl_sub/subscriptions/subscription_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,24 @@ def _initialize_plugins(self) -> List[Plugin]:
-------
List of plugins defined in the subscription, initialized and ready to use.
"""
plugins = [
plugin_type(
initialized_plugins: List[Plugin] = []
for plugin_type, plugin_options in self.plugins.zipped():
plugin = plugin_type(
options=plugin_options,
overrides=self.overrides,
enhanced_download_archive=self.download_archive,
)
for plugin_type, plugin_options in self.plugins.zipped()
]
return [plugin for plugin in plugins if plugin.is_enabled]
if plugin.is_enabled:
initialized_plugins.append(plugin)
else:
# Set any plugin variables to null if it is disabled
for variable_set in plugin_options.added_variables(
unresolved_variables=self.overrides.unresolvable
).values():
nulled_variable_set = {var_name: "" for var_name in variable_set}
self.overrides.add(nulled_variable_set)

return initialized_plugins

@classmethod
def _cleanup_entry_files(cls, entry: Entry):
Expand Down
60 changes: 60 additions & 0 deletions tests/unit/plugins/test_chapters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
from typing import Dict

import pytest

from ytdl_sub.entries.entry import ytdl_sub_chapters_from_comments
from ytdl_sub.subscriptions.subscription import Subscription


@pytest.fixture
def sponsorblock_disabled_dict(output_directory) -> Dict:
return {
"preset": "Jellyfin Music Videos",
"output_options": {"output_directory": output_directory},
"chapters": {
"enable": "{enable_sponsorblock}",
"sponsorblock_categories": [
"outro",
"selfpromo",
"preview",
"interaction",
"sponsor",
"music_offtopic",
"intro",
],
"remove_sponsorblock_categories": "all",
"remove_chapters_regex": [
"Intro",
"Outro",
],
},
"overrides": {
"music_video_artist": "JMC",
"url": "https://your.name.here",
"enable_sponsorblock": "False",
},
}


class TestChapters:
def test_chapters_disabled_respected(
self,
config,
subscription_name,
sponsorblock_disabled_dict,
output_directory,
mock_download_collection_entries,
):
subscription = Subscription.from_dict(
config=config,
preset_name=subscription_name,
preset_dict=sponsorblock_disabled_dict,
)

with mock_download_collection_entries(
is_youtube_channel=False, num_urls=1, is_extracted_audio=False, is_dry_run=True
):
_ = subscription.download(dry_run=True)

script = subscription.overrides.script
assert script.get(ytdl_sub_chapters_from_comments.variable_name).native == ""
2 changes: 1 addition & 1 deletion tests/unit/plugins/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def regex_subscription_dict_base(output_directory):
"filter_include": ["{%not(%is_null(description_website))}"],
"overrides": {
"in_regex_default": "in regex default",
"url": "https://youtube.com/playlist?list=PL5BC0FC26BECA5A35",
"url": "https://your.name.here",
"upload_capture": """{
%regex_capture_many_with_defaults(
upload_date_standardized,
Expand Down

0 comments on commit f2ec26d

Please sign in to comment.