Skip to content

Commit

Permalink
[BUGFIX] Do not perform nested scripting on file path outputs (#1123)
Browse files Browse the repository at this point in the history
Fixes a bug where curly braces in file-names causes scripting to think its a variable that does not exist
  • Loading branch information
jmbannon authored Nov 18, 2024
1 parent 0f542ac commit d3295ce
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 28 deletions.
6 changes: 6 additions & 0 deletions src/ytdl_sub/utils/file_path.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import posixpath
from pathlib import Path
from typing import Tuple

Expand Down Expand Up @@ -59,3 +60,8 @@ def maybe_truncate_file_path(cls, file_path: str) -> str:
return str(Path(file_directory) / cls._truncate_file_name(file_name))

return str(file_path)

@classmethod
def to_native_filepath(cls, file_path: str) -> str:
"""Ensures file paths use the correct separator"""
return os.path.expanduser(file_path.replace(posixpath.sep, os.sep))
24 changes: 5 additions & 19 deletions src/ytdl_sub/validators/file_path_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from pathlib import Path
from typing import Any

from ytdl_sub.script.script import Script
from ytdl_sub.utils.file_path import FilePathTruncater
from ytdl_sub.validators.string_formatter_validators import OverridesStringFormatterValidator
from ytdl_sub.validators.string_formatter_validators import StringFormatterValidator
from ytdl_sub.validators.validators import StringValidator
Expand Down Expand Up @@ -45,29 +45,15 @@ class StringFormatterFileNameValidator(StringFormatterValidator):
_expected_value_type_name = "filepath"

def post_process(self, resolved: str) -> str:
return (
Script(
{
"tmp_var_1": resolved,
"tmp_var_2": "{%to_native_filepath(%truncate_filepath_if_too_long(tmp_var_1))}",
}
)
.resolve()
.get_str("tmp_var_2")
return FilePathTruncater.to_native_filepath(
FilePathTruncater.maybe_truncate_file_path(resolved)
)


class OverridesStringFormatterFilePathValidator(OverridesStringFormatterValidator):
_expected_value_type_name = "static filepath"

def post_process(self, resolved: str) -> str:
return (
Script(
{
"tmp_var_1": resolved,
"tmp_var_2": "{%to_native_filepath(%truncate_filepath_if_too_long(tmp_var_1))}",
}
)
.resolve()
.get_str("tmp_var_2")
return FilePathTruncater.to_native_filepath(
FilePathTruncater.maybe_truncate_file_path(resolved)
)
5 changes: 2 additions & 3 deletions tests/e2e/test_debug_repro.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
from typing import Dict

import pytest
Expand Down Expand Up @@ -44,13 +43,13 @@ class TestReproduce:
def test_debug_log_repro(
self,
default_config,
repro_preset_dict,
debug_log_rerpo,
output_directory,
):
sub = Subscription.from_dict(
config=default_config,
preset_name="repro",
preset_dict=repro_preset_dict,
preset_dict=debug_log_rerpo,
)

transaction_log = sub.download(dry_run=False)
Expand Down
10 changes: 4 additions & 6 deletions tests/unit/validators/test_file_path_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,17 @@ def test_truncates_file_names_successfully(self, ext: str):
if "thumb" not in ext: # do not put . in front of -thumb
ext = f".{ext}" # pytest args with . in the beginning act weird

base_file_name = "s2023.e031701 - 𝗪𝗔𝗥𝗡𝗜𝗡𝗚: LG Secretly Overhaul This OLED Feature on C3 & G3… Should You Buy C2 Instead?"
base_file_name = "s2023.e031701 - 𝗪𝗔𝗥𝗡𝗜𝗡𝗚: {LG} Secretly Overhaul This OLED Feature on C3 & G3… Should You Buy C2 Instead?"

with tempfile.TemporaryDirectory() as temp_dir:
file_path = str(Path(temp_dir) / f"{base_file_name}{ext}")

formatter = StringFormatterFileNameValidator(name="test", value=str(file_path))
truncated_file_path = formatter.post_process(
Script({"file_name": formatter.format_string}).resolve().get_str("file_name")
)
formatter = StringFormatterFileNameValidator(name="test", value="")
truncated_file_path = formatter.post_process(file_path)

assert truncated_file_path == str(
Path(temp_dir)
/ f"s2023.e031701 - 𝗪𝗔𝗥𝗡𝗜𝗡𝗚: LG Secretly Overhaul This OLED Feature on C3 & G3… Should You Buy C2 Instead?{ext}"
/ f"s2023.e031701 - 𝗪𝗔𝗥𝗡𝗜𝗡𝗚: {{LG}} Secretly Overhaul This OLED Feature on C3 & G3… Should You Buy C2 Instead?{ext}"
)

# Ensure it can actually open the file
Expand Down

0 comments on commit d3295ce

Please sign in to comment.