From 451b3564c56a4c69b56c29b44cdb8ab3abe5cf74 Mon Sep 17 00:00:00 2001
From: jbannon <jbann1994@gmail.com>
Date: Wed, 6 Apr 2022 06:09:16 +0000
Subject: [PATCH] return string formatters in overrides dict

---
 tests/unit/entries/conftest.py                | 10 ++-
 tests/unit/entries/test_entry.py              | 25 ++++++--
 .../test_string_formatter_validator.py}       | 58 ++++++++++++++---
 ytdl_subscribe/entries/entry.py               | 20 +++---
 ytdl_subscribe/subscriptions/subscription.py  | 62 +++++++++++++------
 ...ator.py => string_formatter_validators.py} | 18 +++++-
 ytdl_subscribe/validators/base/validators.py  |  5 +-
 .../config/metadata_options/id3_validator.py  |  2 +-
 .../config/metadata_options/nfo_validator.py  |  4 +-
 .../output_options_validator.py               |  2 +-
 .../config/overrides/overrides_validator.py   | 17 ++++-
 11 files changed, 170 insertions(+), 53 deletions(-)
 rename tests/unit/{entries/test_entry_formatter.py => validators/test_string_formatter_validator.py} (52%)
 rename ytdl_subscribe/validators/base/{string_formatter_validator.py => string_formatter_validators.py} (80%)

diff --git a/tests/unit/entries/conftest.py b/tests/unit/entries/conftest.py
index 76e119c9..67456b3d 100644
--- a/tests/unit/entries/conftest.py
+++ b/tests/unit/entries/conftest.py
@@ -1,6 +1,9 @@
 import pytest
 
 from ytdl_subscribe.entries.entry import Entry
+from ytdl_subscribe.validators.base.string_formatter_validators import (
+    StringFormatterValidator,
+)
 
 
 @pytest.fixture
@@ -110,7 +113,12 @@ def validate_entry_dict_contains_valid_formatters():
             expected_string = f"test {value} formatting works"
             format_string = f"test {{{key}}} formatting works"
 
-            assert entry.apply_formatter(format_string) == expected_string
+            assert (
+                entry.apply_formatter(
+                    StringFormatterValidator(name="test", value=format_string)
+                )
+                == expected_string
+            )
 
         return True
 
diff --git a/tests/unit/entries/test_entry.py b/tests/unit/entries/test_entry.py
index 6cd1bbe1..50a35b33 100644
--- a/tests/unit/entries/test_entry.py
+++ b/tests/unit/entries/test_entry.py
@@ -2,6 +2,13 @@ import tempfile
 
 import pytest
 
+from ytdl_subscribe.validators.base.string_formatter_validators import (
+    StringFormatterValidator,
+)
+from ytdl_subscribe.validators.config.overrides.overrides_validator import (
+    OverridesValidator,
+)
+
 
 class TestEntry(object):
     def test_entry_properties(
@@ -30,13 +37,17 @@ class TestEntry(object):
             relative_directory.cleanup()
 
     def test_entry_formatter_single_field(self, mock_entry):
-        format_string = f"prefix {{uid}} suffix"
+        format_string = StringFormatterValidator(
+            name="test", value=f"prefix {{uid}} suffix"
+        )
         expected_string = f"prefix {mock_entry.uid} suffix"
 
         assert mock_entry.apply_formatter(format_string) == expected_string
 
     def test_entry_formatter_duplicate_fields(self, mock_entry):
-        format_string = f"prefix {{upload_year}} {{upload_year}} suffix"
+        format_string = StringFormatterValidator(
+            name="test", value=f"prefix {{upload_year}} {{upload_year}} suffix"
+        )
         expected_string = (
             f"prefix {mock_entry.upload_year} {mock_entry.upload_year} suffix"
         )
@@ -45,9 +56,11 @@ class TestEntry(object):
 
     def test_entry_formatter_override(self, mock_entry):
         new_uid = "my very own uid"
-        overrides = {"uid": new_uid}
+        overrides = OverridesValidator(name="test", value={"uid": new_uid})
 
-        format_string = f"prefix {{uid}} suffix"
+        format_string = StringFormatterValidator(
+            name="test", value=f"prefix {{uid}} suffix"
+        )
         expected_string = f"prefix {new_uid} suffix"
 
         assert (
@@ -64,7 +77,9 @@ class TestEntry(object):
             mock_entry.kwargs(key)
 
     def test_entry_formatter_fails_missing_field(self, mock_entry):
-        format_string = f"prefix {{bah_humbug}} suffix"
+        format_string = StringFormatterValidator(
+            name="test", value=f"prefix {{bah_humbug}} suffix"
+        )
         available_fields = ", ".join(sorted(mock_entry.to_dict().keys()))
         expected_error_msg = f"Format variable 'bah_humbug' does not exist for Entry. Available fields: {available_fields}"
 
diff --git a/tests/unit/entries/test_entry_formatter.py b/tests/unit/validators/test_string_formatter_validator.py
similarity index 52%
rename from tests/unit/entries/test_entry_formatter.py
rename to tests/unit/validators/test_string_formatter_validator.py
index 8586b33b..5d058ae9 100644
--- a/tests/unit/entries/test_entry_formatter.py
+++ b/tests/unit/validators/test_string_formatter_validator.py
@@ -1,6 +1,9 @@
 import pytest
 
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
+    DictFormatterValidator,
+)
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     StringFormatterValidator,
 )
 from ytdl_subscribe.validators.exceptions import ValidationException
@@ -22,14 +25,17 @@ def error_message_unequal_regex_matches_str():
     )
 
 
-class TestEntryFormatter(object):
+class TestStringFormatterValidator(object):
     def test_parse(self):
         format_string = "Here is my {var_one} and {var_two} 💩"
-        assert StringFormatterValidator(
+        validator = StringFormatterValidator(
             name="test_format_variables", value=format_string
-        ).format_variables == ["var_one", "var_two"]
+        )
 
-    def test_parse_no_variables(self):
+        assert validator.format_string == format_string
+        assert validator.format_variables == ["var_one", "var_two"]
+
+    def test_format_variables(self):
         format_string = "No vars 💩"
         assert (
             StringFormatterValidator(
@@ -47,7 +53,7 @@ class TestEntryFormatter(object):
             "Try }var_one} and {var_one}",
         ],
     )
-    def test_parse_fail_uneven_brackets(
+    def test_validate_fail_uneven_brackets(
         self, format_string, error_message_unequal_brackets_str
     ):
         expected_error_msg = (
@@ -60,15 +66,15 @@ class TestEntryFormatter(object):
     @pytest.mark.parametrize(
         "format_string",
         [
-            "Try {var1} no numbers",
-            "Try {VAR1} no caps",
+            "Try {var1} numbers",
+            "Try {VAR1} caps w/numbers",
             "Try {internal{bracket}}",
             "Try }backwards{ facing",
             "Try {var_1}}{",
             "Try {} empty",
         ],
     )
-    def test_parse_fail_variable(
+    def test_validate_fail_bad_variable(
         self, format_string, error_message_unequal_regex_matches_str
     ):
         expected_error_msg = (
@@ -77,3 +83,37 @@ class TestEntryFormatter(object):
 
         with pytest.raises(ValidationException, match=expected_error_msg):
             _ = StringFormatterValidator(name="fail", value=format_string)
+
+    @pytest.mark.parametrize(
+        "format_string, bad_variable",
+        [
+            ("keyword {while}", "while"),
+            ("{try} {valid_var}", "try"),
+        ],
+    )
+    def test_validate_fail_variable_keyword_or_not_identifier(
+        self, format_string, bad_variable
+    ):
+        expected_error_msg = (
+            f"Validation error in fail: "
+            f"'{bad_variable}' is a Python keyword and cannot be used as a variable."
+        )
+
+        with pytest.raises(ValidationException, match=expected_error_msg):
+            _ = StringFormatterValidator(name="fail", value=format_string)
+
+
+class TestDictFormatterValidator(object):
+    def test_validates_values(self):
+        key1_format_string = "string with {variable}"
+        key2_format_string = "no variables"
+        validator = DictFormatterValidator(
+            name="validator",
+            value={"key1": key1_format_string, "key2": key2_format_string},
+        )
+
+        assert validator.dict["key1"].format_string == key1_format_string
+        assert validator.dict["key2"].format_string == key2_format_string
+
+        assert validator.dict["key1"].format_variables == ["variable"]
+        assert validator.dict["key2"].format_variables == []
diff --git a/ytdl_subscribe/entries/entry.py b/ytdl_subscribe/entries/entry.py
index 1f8a5651..e54c60ff 100644
--- a/ytdl_subscribe/entries/entry.py
+++ b/ytdl_subscribe/entries/entry.py
@@ -5,9 +5,12 @@ from typing import Optional
 
 from sanitize_filename import sanitize
 
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     StringFormatterValidator,
 )
+from ytdl_subscribe.validators.config.overrides.overrides_validator import (
+    OverridesValidator,
+)
 
 
 class Entry:
@@ -105,7 +108,9 @@ class Entry:
         }
 
     def apply_formatter(
-        self, format_string: str, overrides: Optional[Dict] = None
+        self,
+        formatter: StringFormatterValidator,
+        overrides: Optional[OverridesValidator] = None,
     ) -> str:
         """
         Perform a string format on the given format string, using the entry's dict for format
@@ -113,13 +118,10 @@ class Entry:
         """
         entry_dict = self.to_dict()
         if overrides:
-            entry_dict = dict(entry_dict, **overrides)
+            # TODO: need to check recursively populate format variables
+            entry_dict = dict(entry_dict, **overrides.dict_with_format_strings)
 
-        field_names = StringFormatterValidator(
-            "TODO: UPDATE", format_string
-        ).format_variables
-
-        for field_name in field_names:
+        for field_name in formatter.format_variables:
             if field_name not in entry_dict:
                 available_fields = ", ".join(sorted(entry_dict.keys()))
                 raise ValueError(
@@ -127,4 +129,4 @@ class Entry:
                     f"for {self.__class__.__name__}. Available fields: {available_fields}"
                 )
 
-        return format_string.format(**entry_dict)
+        return formatter.format_string.format(**entry_dict)
diff --git a/ytdl_subscribe/subscriptions/subscription.py b/ytdl_subscribe/subscriptions/subscription.py
index 57f5dbaa..218d83f4 100644
--- a/ytdl_subscribe/subscriptions/subscription.py
+++ b/ytdl_subscribe/subscriptions/subscription.py
@@ -10,6 +10,9 @@ from PIL import Image
 
 from ytdl_subscribe.downloaders.downloader import Downloader
 from ytdl_subscribe.entries.entry import Entry
+from ytdl_subscribe.validators.base.string_formatter_validators import (
+    StringFormatterValidator,
+)
 from ytdl_subscribe.validators.config.config_options.config_options_validator import (
     ConfigOptionsValidator,
 )
@@ -80,14 +83,31 @@ class Subscription(object):
             ytdl_options=self.ytdl_options.dict,
         )
 
+    def _apply_entry_formatter(
+        self, entry: Entry, formatter: StringFormatterValidator
+    ) -> str:
+        """
+        Parameters
+        ----------
+        entry
+            Entry with values to use in the formatter
+        formatter
+            The formatter itself
+
+        Returns
+        -------
+        The format_string after .format has been called on it using entry and override values
+        """
+        return entry.apply_formatter(formatter=formatter, overrides=self.overrides)
+
     def _post_process_tagging(self, entry: Entry):
         id3_options = self.metadata_options.id3
         audio_file = music_tag.load_file(
             entry.file_path(relative_directory=self.working_directory)
         )
         for tag, tag_formatter in id3_options.tags.dict.items():
-            audio_file[tag] = entry.apply_formatter(
-                format_string=tag_formatter, overrides=self.overrides.dict
+            audio_file[tag] = self._apply_entry_formatter(
+                entry=entry, formatter=tag_formatter
             )
         audio_file.save()
 
@@ -96,24 +116,28 @@ class Subscription(object):
         nfo_options = self.metadata_options.nfo
 
         for tag, tag_formatter in nfo_options.tags.dict.items():
-            nfo[tag] = entry.apply_formatter(
-                format_string=tag_formatter, overrides=self.overrides.dict
-            )
+            nfo[tag] = self._apply_entry_formatter(entry=entry, formatter=tag_formatter)
 
+        # Write the nfo tags to XML with the nfo_root
+        nfo_root = self._apply_entry_formatter(
+            entry=entry, formatter=nfo_options.nfo_root
+        )
         xml = dicttoxml.dicttoxml(
             obj=nfo,
             root=True,  # We assume all NFOs have a root. Maybe we should not?
-            custom_root=nfo_options.nfo_root.value,
+            custom_root=nfo_root,
             attr_type=False,
         )
 
-        nfo_file_name = entry.apply_formatter(
-            format_string=nfo_options.nfo_name.format_string,
-            overrides=self.overrides.dict,
+        nfo_file_name = self._apply_entry_formatter(
+            entry=entry, formatter=nfo_options.nfo_name
         )
-        nfo_file_path = Path(self.output_options.output_directory.value) / Path(
-            nfo_file_name
+        output_directory = self._apply_entry_formatter(
+            entry=entry, formatter=self.output_options.output_directory
         )
+
+        # Save the nfo's XML to file
+        nfo_file_path = Path(output_directory) / Path(nfo_file_name)
         with open(nfo_file_path, "wb") as nfo_file:
             nfo_file.write(xml)
 
@@ -132,13 +156,12 @@ class Subscription(object):
             relative_directory=self.working_directory
         )
 
-        output_directory = entry.apply_formatter(
-            format_string=self.output_options.output_directory.format_string,
-            overrides=self.overrides.dict,
+        output_directory = self._apply_entry_formatter(
+            entry=entry, formatter=self.output_options.output_directory
         )
-        output_file_name = entry.apply_formatter(
-            format_string=self.output_options.file_name.format_string,
-            overrides=self.overrides.dict,
+
+        output_file_name = self._apply_entry_formatter(
+            entry=entry, formatter=self.output_options.file_name
         )
         entry_destination_file_path = Path(output_directory) / Path(output_file_name)
 
@@ -151,9 +174,8 @@ class Subscription(object):
                 relative_directory=self.working_directory
             )
 
-            output_thumbnail_name = entry.apply_formatter(
-                format_string=self.output_options.thumbnail_name.format_string,
-                overrides=self.overrides.dict,
+            output_thumbnail_name = self._apply_entry_formatter(
+                entry=entry, formatter=self.output_options.thumbnail_name
             )
             output_thumbnail_path = Path(output_directory) / Path(output_thumbnail_name)
 
diff --git a/ytdl_subscribe/validators/base/string_formatter_validator.py b/ytdl_subscribe/validators/base/string_formatter_validators.py
similarity index 80%
rename from ytdl_subscribe/validators/base/string_formatter_validator.py
rename to ytdl_subscribe/validators/base/string_formatter_validators.py
index ff14a868..3201571a 100644
--- a/ytdl_subscribe/validators/base/string_formatter_validator.py
+++ b/ytdl_subscribe/validators/base/string_formatter_validators.py
@@ -1,5 +1,6 @@
 import re
 from keyword import iskeyword
+from typing import Dict
 from typing import List
 from typing import final
 
@@ -80,4 +81,19 @@ class DictFormatterValidator(LiteralDictValidator):
         super().__init__(name, value)
 
         for key in self._keys:
-            _ = self._validate_key(key=key, validator=StringFormatterValidator)
+            self._value[key] = self._validate_key(
+                key=key, validator=StringFormatterValidator
+            )
+
+    @property
+    def dict(self) -> Dict[str, StringFormatterValidator]:
+        """Returns dict with string formatter values"""
+        return self._value
+
+    @property
+    def dict_with_format_strings(self) -> Dict[str, str]:
+        """Returns dict with the format strings themselves"""
+        return {
+            key: string_formatter.format_string
+            for key, string_formatter in self.dict.items()
+        }
diff --git a/ytdl_subscribe/validators/base/validators.py b/ytdl_subscribe/validators/base/validators.py
index c33bf553..9c0728f1 100644
--- a/ytdl_subscribe/validators/base/validators.py
+++ b/ytdl_subscribe/validators/base/validators.py
@@ -1,3 +1,4 @@
+import copy
 from abc import ABC
 from typing import Any
 from typing import Dict
@@ -25,7 +26,9 @@ class Validator(ABC):
 
     def __init__(self, name: str, value: Any):
         self._name = name
-        self._value = value
+        self._value = copy.deepcopy(
+            value
+        )  # Always deep copy to avoid editing references
 
         if not isinstance(self._value, self._expected_value_type):
             expected_value_type_name = self._expected_value_type_name or str(
diff --git a/ytdl_subscribe/validators/config/metadata_options/id3_validator.py b/ytdl_subscribe/validators/config/metadata_options/id3_validator.py
index aed8da57..07a93a17 100644
--- a/ytdl_subscribe/validators/config/metadata_options/id3_validator.py
+++ b/ytdl_subscribe/validators/config/metadata_options/id3_validator.py
@@ -1,5 +1,5 @@
 from ytdl_subscribe.validators.base.strict_dict_validator import StrictDictValidator
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     DictFormatterValidator,
 )
 from ytdl_subscribe.validators.base.string_select_validator import StringSelectValidator
diff --git a/ytdl_subscribe/validators/config/metadata_options/nfo_validator.py b/ytdl_subscribe/validators/config/metadata_options/nfo_validator.py
index 20a7a5b2..cc27fed8 100644
--- a/ytdl_subscribe/validators/config/metadata_options/nfo_validator.py
+++ b/ytdl_subscribe/validators/config/metadata_options/nfo_validator.py
@@ -1,8 +1,8 @@
 from ytdl_subscribe.validators.base.strict_dict_validator import StrictDictValidator
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     DictFormatterValidator,
 )
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     StringFormatterValidator,
 )
 
diff --git a/ytdl_subscribe/validators/config/output_options/output_options_validator.py b/ytdl_subscribe/validators/config/output_options/output_options_validator.py
index e06185f8..ea8874e8 100644
--- a/ytdl_subscribe/validators/config/output_options/output_options_validator.py
+++ b/ytdl_subscribe/validators/config/output_options/output_options_validator.py
@@ -1,5 +1,5 @@
 from ytdl_subscribe.validators.base.strict_dict_validator import StrictDictValidator
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     StringFormatterValidator,
 )
 from ytdl_subscribe.validators.base.string_select_validator import StringSelectValidator
diff --git a/ytdl_subscribe/validators/config/overrides/overrides_validator.py b/ytdl_subscribe/validators/config/overrides/overrides_validator.py
index 1abbd06d..c2514615 100644
--- a/ytdl_subscribe/validators/config/overrides/overrides_validator.py
+++ b/ytdl_subscribe/validators/config/overrides/overrides_validator.py
@@ -1,8 +1,11 @@
 import sanitize_filename
 
-from ytdl_subscribe.validators.base.string_formatter_validator import (
+from ytdl_subscribe.validators.base.string_formatter_validators import (
     DictFormatterValidator,
 )
+from ytdl_subscribe.validators.base.string_formatter_validators import (
+    StringFormatterValidator,
+)
 
 
 class OverridesValidator(DictFormatterValidator):
@@ -11,6 +14,14 @@ class OverridesValidator(DictFormatterValidator):
     def __init__(self, name, value):
         super().__init__(name, value)
         for key in self._keys:
-            self._value[f"sanitized_{key}"] = sanitize_filename.sanitize(
-                self._value[key]
+            sanitized_key_name = f"sanitized_{key}"
+            # First, sanitize the format string
+            self._value[sanitized_key_name] = sanitize_filename.sanitize(
+                self._value[key].format_string
+            )
+
+            # Then, convert it into a StringFormatterValidator
+            self._value[sanitized_key_name] = StringFormatterValidator(
+                name="__should_never_fail__",
+                value=self._value[sanitized_key_name],
             )