danvics/ytdl-sub
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[BUGFIX] Smarter subscription validation (#895)

Browse source 
With the right functions, it was possible for subscription validation to raise a false-positive error. This should hopefully resolve that issue and give validation a small performance increase
This commit is contained in:
Jesse Bannon 2024-01-10 18:16:12 -08:00 committed by GitHub
parent 00e4cd8541
commit 2076de3074
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 157 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/ytdl_sub/config/validators/variable_validation.py
View file

@ -13,9 +13,10 @@ from ytdl_sub.config.plugin.preset_plugins import PresetPlugins
from ytdl_sub.config.preset_options import OutputOptions
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.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 +33,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 +73,15 @@ def _override_variables(overrides: Overrides) -> Set[str]:
}
def _entry_variables() -> Set[str]:
return set(list(VARIABLE_SCRIPTS.keys()))
_DUMMY_ENTRY_VARIABLES: Dict[str, str] = {
name: to_variable_dependency_format_string(
# pylint: disable=protected-access
script=BASE_SCRIPT,
parsed_format_string=BASE_SCRIPT._variables[name]
# pylint: enable=protected-access
)
for name in BASE_SCRIPT.variable_names
}
class VariableValidation:
@ -97,12 +105,12 @@ class VariableValidation:
"""
Do some gymnastics to initialize the Overrides script.
"""
entry_variables = _entry_variables()
override_variables = _override_variables(overrides)
# 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(_DUMMY_ENTRY_VARIABLES.keys()) | override_variables
plugin_variables: Set[str] = set()
for (
plugin_options,
@ -125,6 +133,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 +146,11 @@ 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)
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)
| _DUMMY_ENTRY_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(

53
src/ytdl_sub/script/script.py
View file

@ -248,6 +248,52 @@ class Script:
for variable_name, resolved in resolved_variables.items():
self._variables[variable_name] = SyntaxTree(ast=[resolved])
def _recursive_get_unresolved_output_filter_variables(
self, current_var: SyntaxTree, subset_to_resolve: Set[str], unresolvable: Set[Variable]
) -> Set[str]:
for var_dep in current_var.variables:
if var_dep in unresolvable:
raise ScriptVariableNotResolved(
f"Output filter variable contains the variable {var_dep} "
f"which is set as unresolvable"
)
subset_to_resolve.add(var_dep.name)
subset_to_resolve |= self._recursive_get_unresolved_output_filter_variables(
current_var=self._variables[var_dep.name],
subset_to_resolve=subset_to_resolve,
unresolvable=unresolvable,
)
return subset_to_resolve
def _get_unresolved_output_filter(
self,
unresolved: Dict[Variable, SyntaxTree],
output_filter: Set[str],
unresolvable: Set[Variable],
) -> Dict[Variable, SyntaxTree]:
"""
When an output filter is applied, only a subset of variables that the filter
depends on need to be resolved.
"""
subset_to_resolve: Set[str] = set()
for output_filter_variable in output_filter:
subset_to_resolve.add(output_filter_variable)
if output_filter_variable not in self._variables:
raise ScriptVariableNotResolved(
"Tried to specify an output filter variable that does not exist"
)
subset_to_resolve |= self._recursive_get_unresolved_output_filter_variables(
current_var=self._variables[output_filter_variable],
subset_to_resolve=subset_to_resolve,
unresolvable=unresolvable,
)
return {var: syntax for var, syntax in unresolved.items() if var.name in subset_to_resolve}
def _resolve(
self,
pre_resolved: Optional[Dict[str, Resolvable]] = None,
@ -288,6 +334,13 @@ class Script:
if Variable(name) not in unresolved_filter
}
if output_filter:
unresolved = self._get_unresolved_output_filter(
unresolved=unresolved,
output_filter=output_filter,
unresolvable=unresolvable,
)
while unresolved:
unresolved_count: int = len(unresolved)

8
src/ytdl_sub/utils/scriptable.py
View file

@ -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(
dict(ScriptUtils.add_sanitized_variables(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

62
src/ytdl_sub/validators/string_formatter_validators.py
View file

@ -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 = {f"%{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

28
tests/unit/config/test_preset.py
View file

@ -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,