From 7f61058d0ebb286449d4ca70503c9bb74d47dbfa Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 10 Jan 2024 16:25:41 -0800 Subject: [PATCH] [BUGFIX] Mock all variable definitions when validating --- .../config/validators/variable_validation.py | 34 +++++----- src/ytdl_sub/utils/scriptable.py | 8 +-- .../validators/string_formatter_validators.py | 62 ++++++++++++++++--- tests/e2e/youtube/test_video.py | 3 +- tests/unit/config/test_preset.py | 28 +++++++-- 5 files changed, 104 insertions(+), 31 deletions(-) diff --git a/src/ytdl_sub/config/validators/variable_validation.py b/src/ytdl_sub/config/validators/variable_validation.py index cc554a70..8e95ad1c 100644 --- a/src/ytdl_sub/config/validators/variable_validation.py +++ b/src/ytdl_sub/config/validators/variable_validation.py @@ -15,7 +15,10 @@ from ytdl_sub.config.validators.options import OptionsValidator from ytdl_sub.downloaders.url.validators import MultiUrlValidator from ytdl_sub.entries.script.variable_definitions import VARIABLE_SCRIPTS from ytdl_sub.entries.variables.override_variables import SubscriptionVariables +from ytdl_sub.script.parser import parse from ytdl_sub.script.script import Script +from ytdl_sub.utils.scriptable import BASE_SCRIPT +from ytdl_sub.validators.string_formatter_validators import to_variable_dependency_format_string from ytdl_sub.validators.string_formatter_validators import validate_formatters @@ -32,10 +35,10 @@ def _add_dummy_overrides(overrides: Overrides) -> Dict[str, str]: # Have the dummy override variable contain all variable deps that it uses in the string dummy_overrides: Dict[str, str] = {} for override_name in _override_variables(overrides): - dummy_overrides[override_name] = "" # pylint: disable=protected-access - for variable_dependency in overrides.script._variables[override_name].variables: - dummy_overrides[override_name] += f"{{ {variable_dependency.name } }}" + dummy_overrides[override_name] = to_variable_dependency_format_string( + script=overrides.script, parsed_format_string=overrides.script._variables[override_name] + ) # pylint: enable=protected-access return dummy_overrides @@ -72,8 +75,13 @@ def _override_variables(overrides: Overrides) -> Set[str]: } -def _entry_variables() -> Set[str]: - return set(list(VARIABLE_SCRIPTS.keys())) +def _entry_variables() -> Dict[str, str]: + return { + name: to_variable_dependency_format_string( + script=BASE_SCRIPT, parsed_format_string=BASE_SCRIPT._variables[name] + ) + for name in BASE_SCRIPT.variable_names + } class VariableValidation: @@ -102,7 +110,8 @@ class VariableValidation: # Set resolved variables as all entry + override variables # at this point to generate every possible added/modified variable - self.resolved_variables = entry_variables | override_variables + self.resolved_variables = set(entry_variables.keys()) | override_variables + plugin_variables: Set[str] = set() for ( plugin_options, @@ -125,6 +134,7 @@ class VariableValidation: # Set unresolved as variables that are added but do not exist as # entry/override variables since they are created at run-time self.unresolved_variables |= added_variables | modified_variables + plugin_variables |= added_variables | modified_variables # Then update resolved variables to reflect that self.resolved_variables -= self.unresolved_variables @@ -137,10 +147,10 @@ class VariableValidation: ) # copy the script and mock entry variables - self.script = copy.deepcopy(overrides.script).add(_add_dummy_variables(entry_variables)) + self.script = copy.deepcopy(overrides.script).add(entry_variables) self.script.add( - variables=_add_dummy_overrides(overrides=overrides), - unresolvable=self.unresolved_variables, + variables=_add_dummy_overrides(overrides=overrides) + | _add_dummy_variables(variables=plugin_variables) ) return self @@ -158,7 +168,6 @@ class VariableValidation: resolved_variables = added_variables | modified_variables - self.script.add(_add_dummy_variables(resolved_variables)) self.resolved_variables |= resolved_variables self.unresolved_variables -= resolved_variables @@ -177,13 +186,10 @@ class VariableValidation: ): self._add_variables(PluginOperation.MODIFY_ENTRY_METADATA, options=plugin_options) - self._update_script() for plugin_options in PluginMapping.order_options_by( self.plugins.zipped(), PluginOperation.MODIFY_ENTRY ): - added = self._add_variables(PluginOperation.MODIFY_ENTRY, options=plugin_options) - if added: - self._update_script() + self._add_variables(PluginOperation.MODIFY_ENTRY, options=plugin_options) # Validate that any formatter in the plugin options can resolve validate_formatters( diff --git a/src/ytdl_sub/utils/scriptable.py b/src/ytdl_sub/utils/scriptable.py index b1dc20e7..4a136468 100644 --- a/src/ytdl_sub/utils/scriptable.py +++ b/src/ytdl_sub/utils/scriptable.py @@ -14,10 +14,8 @@ from ytdl_sub.script.utils.exceptions import RuntimeException from ytdl_sub.utils.exceptions import StringFormattingException from ytdl_sub.utils.script import ScriptUtils -_BASE_SCRIPT: Script = Script( - ScriptUtils.add_sanitized_variables( - dict(copy.deepcopy(VARIABLE_SCRIPTS), **copy.deepcopy(CUSTOM_FUNCTION_SCRIPTS)) - ) +BASE_SCRIPT: Script = Script( + ScriptUtils.add_sanitized_variables(dict(VARIABLE_SCRIPTS, **CUSTOM_FUNCTION_SCRIPTS)) ) @@ -37,7 +35,7 @@ class Scriptable(ABC): """ Initializes with base values """ - self._script = copy.deepcopy(_BASE_SCRIPT) + self._script = copy.deepcopy(BASE_SCRIPT) self._unresolvable = copy.deepcopy(UNRESOLVED_VARIABLES) @property diff --git a/src/ytdl_sub/validators/string_formatter_validators.py b/src/ytdl_sub/validators/string_formatter_validators.py index 7957169d..5c776d43 100644 --- a/src/ytdl_sub/validators/string_formatter_validators.py +++ b/src/ytdl_sub/validators/string_formatter_validators.py @@ -6,8 +6,10 @@ from typing import final from ytdl_sub.entries.script.variable_definitions import VARIABLES from ytdl_sub.script.parser import parse from ytdl_sub.script.script import Script +from ytdl_sub.script.types.syntax_tree import SyntaxTree +from ytdl_sub.script.utils.exceptions import RuntimeException +from ytdl_sub.script.utils.exceptions import ScriptVariableNotResolved from ytdl_sub.script.utils.exceptions import UserException -from ytdl_sub.script.utils.exceptions import VariableDoesNotExist from ytdl_sub.utils.exceptions import StringFormattingVariableNotFoundException from ytdl_sub.validators.validators import DictValidator from ytdl_sub.validators.validators import ListValidator @@ -142,21 +144,67 @@ class OverridesDictFormatterValidator(DictFormatterValidator): _key_validator = OverridesStringFormatterValidator +def to_variable_dependency_format_string(script: Script, parsed_format_string: SyntaxTree) -> str: + """ + Create a dummy format string that contains all variable deps as a string. + """ + dummy_format_string = "" + for var in parsed_format_string.variables: + dummy_format_string += f"{{ {var.name} }}" + # pylint: disable=protected-access + for variable_dependency in script._variables[var.name].variables: + dummy_format_string += f"{{ {variable_dependency.name} }}" + # pylint: enable=protected-access + return dummy_format_string + + def _validate_formatter( mock_script: Script, unresolved_variables: Set[str], formatter_validator: Union[StringFormatterValidator, OverridesStringFormatterValidator], ) -> None: - try: - unresolvable = unresolved_variables - if isinstance(formatter_validator, OverridesStringFormatterValidator): - unresolvable = unresolved_variables.union({VARIABLES.entry_metadata.variable_name}) + is_static_formatter = False + unresolvable = unresolved_variables + if isinstance(formatter_validator, OverridesStringFormatterValidator): + is_static_formatter = True + unresolvable = unresolved_variables.union({VARIABLES.entry_metadata.variable_name}) + parsed = parse( + text=formatter_validator.format_string, + ) + variable_names = {var.name for var in parsed.variables} + custom_function_names = {func.name for func in parsed.custom_functions} + + if not variable_names.issubset(mock_script.variable_names): + raise StringFormattingVariableNotFoundException( + "contains the following variables that do not exist: " + f"{', '.join(sorted(variable_names - mock_script.variable_names))}" + ) + if not custom_function_names.issubset(mock_script.function_names): + raise StringFormattingVariableNotFoundException( + "contains the following custom functions that do not exist: " + f"{', '.join(sorted(custom_function_names - mock_script.function_names))}" + ) + if unresolved := variable_names.intersection(unresolvable): + raise StringFormattingVariableNotFoundException( + "contains the following variables that are unresolved when executing this " + f"formatter: {', '.join(sorted(unresolved))}" + ) + try: mock_script.resolve_once( - {"tmp_var": formatter_validator.format_string}, + { + "tmp_var": to_variable_dependency_format_string( + script=mock_script, parsed_format_string=parsed + ) + }, unresolvable=unresolvable, ) - except VariableDoesNotExist as exc: + except RuntimeException as exc: + if isinstance(exc, ScriptVariableNotResolved) and is_static_formatter: + raise StringFormattingVariableNotFoundException( + "static formatters must contain variables that have no dependency to " + "entry variables" + ) from exc raise StringFormattingVariableNotFoundException(exc) from exc diff --git a/tests/e2e/youtube/test_video.py b/tests/e2e/youtube/test_video.py index f2c78bb8..78201fa3 100644 --- a/tests/e2e/youtube/test_video.py +++ b/tests/e2e/youtube/test_video.py @@ -31,6 +31,7 @@ def single_video_preset_dict_old_video_tags_format(output_directory): "title": "{title}", } }, + "filter_include": ["{ %ne( %map_get(entry_metadata, 'artist', null), null )}"], "overrides": { "music_video_artist": "JMC", "music_video_directory": output_directory, @@ -88,7 +89,7 @@ def single_tv_show_video_nulled_values_preset_dict(output_directory): }, "overrides": { "url": "https://www.youtube.com/@ProjectZombie603", - "tv_show_name": "Project Zombie", + "tv_show_name": "Project Zombie {title}", "tv_show_directory": output_directory, }, } diff --git a/tests/unit/config/test_preset.py b/tests/unit/config/test_preset.py index dff51a50..8afedd35 100644 --- a/tests/unit/config/test_preset.py +++ b/tests/unit/config/test_preset.py @@ -129,7 +129,7 @@ class TestPreset: ): with pytest.raises( StringFormattingVariableNotFoundException, - match="Variable dne_var does not exist.", + match="contains the following variables that do not exist: dne_var", ): _ = Preset( config=config_file, @@ -145,7 +145,7 @@ class TestPreset: ): with pytest.raises( StringFormattingVariableNotFoundException, - match="Variable dne_var does not exist", + match="contains the following variables that do not exist: dne_var", ): _ = Preset( config=config_file, @@ -161,7 +161,7 @@ class TestPreset: ): with pytest.raises( StringFormattingVariableNotFoundException, - match="Variable dne_var does not exist", + match="contains the following variables that do not exist: dne_var", ): _ = Preset( config=config_file, @@ -182,7 +182,7 @@ class TestPreset: ): with pytest.raises( StringFormattingVariableNotFoundException, - match="Variable dne_var does not exist", + match="contains the following variables that do not exist: dne_var", ): _ = Preset( config=config_file, @@ -198,6 +198,26 @@ class TestPreset: }, ) + def test_preset_error__dict_override_variable_not_static( + self, config_file, output_options, youtube_video + ): + with pytest.raises( + StringFormattingVariableNotFoundException, + match="static formatters must contain variables that " + "have no dependency to entry variables", + ): + _ = Preset( + config=config_file, + name="test", + value={ + "download": youtube_video, + "output_options": { + "output_directory": "{title}", + "file_name": "{uid}", + }, + }, + ) + def test_preset_with_multi_url__contains_empty_url(self, config_file, output_options): _ = Preset( config=config_file,