From f2ec26d77efc9490572b2442a9eb77dd67660281 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Sun, 29 Sep 2024 22:38:26 -0700 Subject: [PATCH] [BUGFIX] Fix variable collision when disabling plugins (#1072) Fixes https://github.com/jmbannon/ytdl-sub/issues/1071 --- .../config/plugin/plugin_operation.py | 1 - src/ytdl_sub/config/validators/options.py | 2 - .../config/validators/variable_validation.py | 4 -- src/ytdl_sub/downloaders/url/validators.py | 21 +++---- src/ytdl_sub/plugins/chapters.py | 2 - src/ytdl_sub/plugins/split_by_chapters.py | 2 - src/ytdl_sub/plugins/subtitles.py | 2 - .../subscriptions/subscription_download.py | 19 ++++-- tests/unit/plugins/test_chapters.py | 60 +++++++++++++++++++ tests/unit/plugins/test_filter.py | 2 +- 10 files changed, 84 insertions(+), 31 deletions(-) create mode 100644 tests/unit/plugins/test_chapters.py diff --git a/src/ytdl_sub/config/plugin/plugin_operation.py b/src/ytdl_sub/config/plugin/plugin_operation.py index d3323aca..2c187f30 100644 --- a/src/ytdl_sub/config/plugin/plugin_operation.py +++ b/src/ytdl_sub/config/plugin/plugin_operation.py @@ -2,7 +2,6 @@ from enum import Enum class PluginOperation(Enum): - ANY = -2 DOWNLOADER = -1 MODIFY_ENTRY_METADATA = 0 MODIFY_ENTRY = 1 diff --git a/src/ytdl_sub/config/validators/options.py b/src/ytdl_sub/config/validators/options.py index 76b6d3ad..5fbdc14d 100644 --- a/src/ytdl_sub/config/validators/options.py +++ b/src/ytdl_sub/config/validators/options.py @@ -42,9 +42,7 @@ class OptionsValidator(Validator, ABC): 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. diff --git a/src/ytdl_sub/config/validators/variable_validation.py b/src/ytdl_sub/config/validators/variable_validation.py index 4d5cefc6..60488394 100644 --- a/src/ytdl_sub/config/validators/variable_validation.py +++ b/src/ytdl_sub/config/validators/variable_validation.py @@ -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) @@ -167,9 +165,7 @@ class VariableValidation: 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()) diff --git a/src/ytdl_sub/downloaders/url/validators.py b/src/ytdl_sub/downloaders/url/validators.py index a5a2616c..1f22faa1 100644 --- a/src/ytdl_sub/downloaders/url/validators.py +++ b/src/ytdl_sub/downloaders/url/validators.py @@ -320,24 +320,21 @@ class MultiUrlValidator(OptionsValidator): 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)} diff --git a/src/ytdl_sub/plugins/chapters.py b/src/ytdl_sub/plugins/chapters.py index 55b7ac83..0394a0fd 100644 --- a/src/ytdl_sub/plugins/chapters.py +++ b/src/ytdl_sub/plugins/chapters.py @@ -207,9 +207,7 @@ class ChaptersOptions(ToggleableOptionsDictValidator): 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}} diff --git a/src/ytdl_sub/plugins/split_by_chapters.py b/src/ytdl_sub/plugins/split_by_chapters.py index e7a9f831..20722672 100644 --- a/src/ytdl_sub/plugins/split_by_chapters.py +++ b/src/ytdl_sub/plugins/split_by_chapters.py @@ -82,9 +82,7 @@ class SplitByChaptersOptions(OptionsDictValidator): def added_variables( self, - resolved_variables: Set[str], unresolved_variables: Set[str], - plugin_op: PluginOperation, ) -> Dict[PluginOperation, Set[str]]: return { PluginOperation.MODIFY_ENTRY: { diff --git a/src/ytdl_sub/plugins/subtitles.py b/src/ytdl_sub/plugins/subtitles.py index 7be3d8f3..ecf32a1d 100644 --- a/src/ytdl_sub/plugins/subtitles.py +++ b/src/ytdl_sub/plugins/subtitles.py @@ -132,9 +132,7 @@ class SubtitleOptions(ToggleableOptionsDictValidator): def added_variables( self, - resolved_variables: Set[str], unresolved_variables: Set[str], - plugin_op: PluginOperation, ) -> Dict[PluginOperation, Set[str]]: """ Returns diff --git a/src/ytdl_sub/subscriptions/subscription_download.py b/src/ytdl_sub/subscriptions/subscription_download.py index 95108ece..790ffd26 100644 --- a/src/ytdl_sub/subscriptions/subscription_download.py +++ b/src/ytdl_sub/subscriptions/subscription_download.py @@ -190,15 +190,24 @@ class SubscriptionDownload(BaseSubscription, ABC): ------- 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): diff --git a/tests/unit/plugins/test_chapters.py b/tests/unit/plugins/test_chapters.py new file mode 100644 index 00000000..8ba3f00a --- /dev/null +++ b/tests/unit/plugins/test_chapters.py @@ -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 == "" diff --git a/tests/unit/plugins/test_filter.py b/tests/unit/plugins/test_filter.py index 3f214cf0..9f9e9883 100644 --- a/tests/unit/plugins/test_filter.py +++ b/tests/unit/plugins/test_filter.py @@ -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,