diff --git a/src/ytdl_sub/config/overrides.py b/src/ytdl_sub/config/overrides.py index 7d953e1f..a1ef2f81 100644 --- a/src/ytdl_sub/config/overrides.py +++ b/src/ytdl_sub/config/overrides.py @@ -9,9 +9,9 @@ from ytdl_sub.entries.entry import Entry from ytdl_sub.entries.script.variable_definitions import VARIABLES from ytdl_sub.entries.variables.override_variables import SUBSCRIPTION_NAME from ytdl_sub.entries.variables.override_variables import OverrideVariables -from ytdl_sub.script.functions import Functions from ytdl_sub.script.parser import parse from ytdl_sub.script.script import Script +from ytdl_sub.utils.exceptions import ValidationException from ytdl_sub.utils.script import ScriptUtils from ytdl_sub.utils.scriptable import Scriptable from ytdl_sub.validators.string_formatter_validators import DictFormatterValidator @@ -56,28 +56,48 @@ class Overrides(DictFormatterValidator, Scriptable): Scriptable.__init__(self) for key in self._keys: - if OverrideVariables.is_override_variable_name(key): - raise self._validation_exception( - f"Override variable with name {key} cannot be used since it is a" - " built-in ytdl-sub override variable name." - ) - - if key in self.script.variable_names: - raise self._validation_exception( - f"Override variable with name {key} cannot be used since it is a" - " built-in ytdl-sub entry variable name." - ) - - if key in self.script.function_names or ( - key.startswith("%") and Functions.is_built_in(key[1:]) - ): - raise self._validation_exception( - f"Override function definition with name {key} cannot be used since it is a" - " built-in ytdl-sub function name." - ) + self.ensure_variable_name_valid(key) self.unresolvable.add(VARIABLES.entry_metadata.variable_name) + def ensure_added_plugin_variable_valid(self, added_variable: str) -> bool: + """ + Returns False if the variable exists as a non-override. + + Raises + ------ + ValidationException + If the variable is already added as an override variable. + """ + try: + self.ensure_variable_name_valid(added_variable) + except ValidationException: + return False + + if added_variable in self.keys: + raise self._validation_exception( + f"Override variable with name {added_variable} cannot be used since it is" + " added by a plugin." + ) + + return True + + def ensure_variable_name_valid(self, name: str) -> None: + """ + Ensures the variable name does not collide with any entry variables or built-in functions. + """ + if OverrideVariables.is_entry_variable_name(name): + raise self._validation_exception( + f"Override variable with name {name} cannot be used since it is a" + " built-in ytdl-sub entry variable name." + ) + + if OverrideVariables.is_function_name(name): + raise self._validation_exception( + f"Override function definition with name {name} cannot be used since it is" + " a built-in ytdl-sub function name." + ) + def initial_variables( self, unresolved_variables: Optional[Dict[str, str]] = None ) -> Dict[str, str]: diff --git a/src/ytdl_sub/config/validators/variable_validation.py b/src/ytdl_sub/config/validators/variable_validation.py index d8155208..d80e7601 100644 --- a/src/ytdl_sub/config/validators/variable_validation.py +++ b/src/ytdl_sub/config/validators/variable_validation.py @@ -29,14 +29,17 @@ def _add_dummy_variables(variables: Iterable[str]) -> Dict[str, str]: def _get_added_and_modified_variables( plugins: PresetPlugins, downloader_options: MultiUrlValidator, resolved_variables: Set[str] -) -> Tuple[Set[str], Set[str]]: - added_variables: Set[str] = set() - modified_variables: Set[str] = set() - +) -> Iterable[Tuple[OptionsValidator, Set[str], Set[str]]]: + """ + Iterates and returns the plugin options, added variables, modified variables + """ options: List[OptionsValidator] = plugins.plugin_options options.append(downloader_options) for plugin_options in options: + added_variables: Set[str] = set() + modified_variables: Set[str] = set() + for plugin_added_variables in plugin_options.added_variables( resolved_variables=resolved_variables, unresolved_variables=set(), @@ -44,9 +47,9 @@ def _get_added_and_modified_variables( added_variables |= set(plugin_added_variables) for plugin_modified_variables in plugin_options.modified_variables().values(): - modified_variables |= plugin_modified_variables + modified_variables = plugin_modified_variables - return added_variables, modified_variables + yield plugin_options, added_variables, modified_variables def _override_variables(overrides: Overrides) -> Set[str]: @@ -82,24 +85,31 @@ 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 - added_variables, modified_variables = _get_added_and_modified_variables( + + for ( + plugin_options, + added_variables, + modified_variables, + ) in _get_added_and_modified_variables( plugins=self.plugins, downloader_options=self.downloader_options, resolved_variables=self.resolved_variables, - ) + ): - for added_variable in added_variables: - if added_variable in overrides.keys: - # pylint: disable=protected-access - raise overrides._validation_exception( - f"Override variable with name {added_variable} cannot be used since it is a" - " built-in ytdl-sub variable added by a plugin." - ) - # pylint: enable=protected-access + for added_variable in added_variables: + if not overrides.ensure_added_plugin_variable_valid(added_variable=added_variable): + # pylint: disable=protected-access + raise plugin_options._validation_exception( + f"Cannot use the variable name {added_variable} because it exists as a" + " built-in ytdl-sub variable name." + ) + # pylint: enable=protected-access + + # 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 - # Set unresolved as variables that are added but do not exist as entry/override variables # Then update resolved variables to reflect that - self.unresolved_variables = added_variables | modified_variables self.resolved_variables -= self.unresolved_variables # Initialize overrides with unresolved variables + modified variables to throw an error. diff --git a/src/ytdl_sub/entries/variables/override_variables.py b/src/ytdl_sub/entries/variables/override_variables.py index 4bb75e1a..b16bf68a 100644 --- a/src/ytdl_sub/entries/variables/override_variables.py +++ b/src/ytdl_sub/entries/variables/override_variables.py @@ -1,4 +1,6 @@ -import re +from ytdl_sub.entries.script.function_scripts import CUSTOM_FUNCTION_SCRIPTS +from ytdl_sub.entries.script.variable_scripts import VARIABLE_SCRIPTS +from ytdl_sub.script.functions import Functions SUBSCRIPTION_NAME = "subscription_name" SUBSCRIPTION_VALUE = "subscription_value" @@ -61,24 +63,21 @@ class OverrideVariables: return f"subscription_value_{index + 1}" @classmethod - def is_override_variable_name(cls, variable_name: str): + def is_entry_variable_name(cls, name: str) -> bool: """ Returns ------- - True of a variable name collides with an override variable name. False otherwise. + True if the name is an entry variable name. False otherwise. """ - if variable_name in ( - SUBSCRIPTION_NAME, - SUBSCRIPTION_VALUE, - SUBSCRIPTION_MAP, - SUBSCRIPTION_ARRAY, - ): - return True - - if re.fullmatch(r"^subscription_value_(\d+)$", variable_name): - return True - - if re.fullmatch(r"^subscription_indent_(\d+)$", variable_name): - return True + return name in VARIABLE_SCRIPTS + @classmethod + def is_function_name(cls, name: str) -> bool: + """ + Returns + ------- + True if the name is a function name (either built-in or script). False otherwise. + """ + if name.startswith("%"): + return name in CUSTOM_FUNCTION_SCRIPTS or Functions.is_built_in(name[1:]) return False diff --git a/tests/unit/config/test_preset.py b/tests/unit/config/test_preset.py index 1032fb3e..c3273787 100644 --- a/tests/unit/config/test_preset.py +++ b/tests/unit/config/test_preset.py @@ -208,29 +208,6 @@ class TestPreset: }, ) - @pytest.mark.parametrize( - "override_variable_name", ["subscription_name", "subscription_value_3", "subscription_map"] - ) - def test_preset_error_override_variable_collides_with_override( - self, config_file, output_options, youtube_video, override_variable_name: str - ): - with pytest.raises( - ValidationException, - match=re.escape( - f"Override variable with name {override_variable_name} cannot be used since" - " it is a built-in ytdl-sub override variable name." - ), - ): - _ = Preset( - config=config_file, - name="test", - value={ - "download": youtube_video, - "output_options": {"output_directory": "dir", "file_name": "{dne_var}"}, - "overrides": {override_variable_name: "fail"}, - }, - ) - @pytest.mark.parametrize( "entry_variable_name", [ @@ -267,7 +244,7 @@ class TestPreset: ValidationException, match=re.escape( f"Override variable with name subtitles_ext cannot be used since" - " it is a built-in ytdl-sub variable added by a plugin." + " it is added by a plugin." ), ): _ = Preset( @@ -311,3 +288,48 @@ class TestPreset: "overrides": {function_name: "fail"}, }, ) + + def test_preset_error_override_added_variable_collides_with_built_in( + self, config_file, output_options + ): + with pytest.raises( + ValidationException, + match=re.escape( + "Cannot use the variable name title because it exists as a " + "built-in ytdl-sub variable name." + ), + ): + _ = Preset( + config=config_file, + name="test", + value={ + "download": { + "url": "youtube.com/watch?v=123abc", + "variables": {"title": "nope"}, + }, + "output_options": {"output_directory": "dir", "file_name": "acjk"}, + }, + ) + + def test_preset_error_override_added_variable_collides_with_override( + self, config_file, output_options + ): + with pytest.raises( + ValidationException, + match=re.escape( + "Override variable with name the_bad_one cannot be used since " + "it is added by a plugin." + ), + ): + _ = Preset( + config=config_file, + name="test", + value={ + "download": { + "url": "youtube.com/watch?v=123abc", + "variables": {"the_bad_one": "should error"}, + }, + "output_options": {"output_directory": "dir", "file_name": "acjk"}, + "overrides": {"the_bad_one": "ack"}, + }, + )