From 4b6add06e1478bf194171e303bbf6102bd126af5 Mon Sep 17 00:00:00 2001
From: Jesse Bannon <jbann1994@gmail.com>
Date: Thu, 29 Sep 2022 00:09:56 -0700
Subject: [PATCH] [BUGFIX] Fix nested parents with multiple parents (#255)

---
 src/ytdl_sub/config/preset.py    | 76 +++++++++++++++++++-------------
 tests/unit/config/test_preset.py | 10 ++++-
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/src/ytdl_sub/config/preset.py b/src/ytdl_sub/config/preset.py
index 89c5b74e..d046d62b 100644
--- a/src/ytdl_sub/config/preset.py
+++ b/src/ytdl_sub/config/preset.py
@@ -277,46 +277,60 @@ class Preset(StrictDictValidator):
             for validator_value in validator.dict.values():
                 self.__validate_override_string_formatter_validator(validator_value)
 
+    def _get_presets_to_merge(
+        self, parent_presets: str | List[str], seen_presets: List[str], config: ConfigFile
+    ) -> List[Dict]:
+        presets_to_merge: List[Dict] = []
+
+        if isinstance(parent_presets, str):
+            parent_presets = [parent_presets]
+
+        for parent_preset in reversed(parent_presets):
+            # Make sure we do not hit an infinite loop
+            if parent_preset in seen_presets:
+                raise self._validation_exception(
+                    f"preset loop detected with the preset '{parent_preset}'"
+                )
+
+            # Make sure the parent preset actually exists
+            if parent_preset not in config.presets.keys:
+                raise self._validation_exception(
+                    f"preset '{parent_preset}' does not exist in the provided config. "
+                    f"Available presets: {', '.join(config.presets.keys)}"
+                )
+
+            parent_preset_dict = copy.deepcopy(config.presets.dict[parent_preset])
+            presets_to_merge.append(parent_preset_dict)
+
+            if "preset" in parent_preset_dict:
+                presets_to_merge.extend(
+                    self._get_presets_to_merge(
+                        parent_presets=parent_preset_dict["preset"],
+                        seen_presets=seen_presets + [parent_preset],
+                        config=config,
+                    )
+                )
+
+        return presets_to_merge
+
     def __merge_parent_preset_dicts_if_present(self, config: ConfigFile):
         parent_preset_validator = self._validate_key_if_present(
             key="preset", validator=StringListValidator
         )
-
         if parent_preset_validator is None:
             return
 
-        presets_to_merge: List[Dict] = []
-        for parent_preset in [preset.value for preset in parent_preset_validator.list]:
-            sub_parent_presets: Dict[str, Dict] = {}
-
-            while parent_preset:
-                # Make sure the parent preset actually exists
-                if parent_preset not in config.presets.keys:
-                    raise self._validation_exception(
-                        f"preset '{parent_preset}' does not exist in the provided config. "
-                        f"Available presets: {', '.join(config.presets.keys)}"
-                    )
-
-                # Make sure we do not hit an infinite loop
-                if parent_preset in sub_parent_presets:
-                    raise self._validation_exception(
-                        f"preset loop detected with the preset '{parent_preset}'"
-                    )
-
-                parent_preset_dict = copy.deepcopy(config.presets.dict[parent_preset])
-
-                sub_parent_presets[parent_preset] = parent_preset_dict
-                parent_preset = parent_preset_dict.get("preset")
-
-            # Extend reversed, so top-most parents are first
-            if sub_parent_presets:
-                presets_to_merge.extend(reversed(sub_parent_presets.values()))
-
-        # Append this preset (the subscription) last
-        presets_to_merge.append(copy.deepcopy(self._value))
+        # Get list of all parent presets in depth-first search order, beginning with this preset
+        presets_to_merge: List[Dict] = [copy.deepcopy(self._value)] + self._get_presets_to_merge(
+            parent_presets=[preset.value for preset in parent_preset_validator.list],
+            seen_presets=[],
+            config=config,
+        )
 
         # Merge all presets
-        self._value = mergedeep.merge({}, *presets_to_merge, strategy=mergedeep.Strategy.ADDITIVE)
+        self._value = mergedeep.merge(
+            {}, *reversed(presets_to_merge), strategy=mergedeep.Strategy.ADDITIVE
+        )
 
     def __init__(self, config: ConfigFile, name: str, value: Any):
         super().__init__(name=name, value=value)
diff --git a/tests/unit/config/test_preset.py b/tests/unit/config/test_preset.py
index bb1d164e..3d469f1a 100644
--- a/tests/unit/config/test_preset.py
+++ b/tests/unit/config/test_preset.py
@@ -32,6 +32,7 @@ def config_file() -> ConfigFile:
                         "tags": {"key-2": "preset_2", "key-3": "preset_2"},
                     }
                 },
+                "parent_preset_3": {"preset": ["parent_preset_1", "parent_preset_2"]},
                 "preset_self_loop": {"preset": "preset_self_loop"},
                 "preset_loop_0": {"preset": "preset_loop_1"},
                 "preset_loop_1": {"preset": "preset_loop_0"},
@@ -113,12 +114,17 @@ class TestPreset:
 
         assert tags_string_dict == {"key-1": "preset_0", "key-2": "this-preset"}
 
-    def test_preset_multiple_parents(self, config_file, output_options, youtube_video):
+    @pytest.mark.parametrize(
+        "preset_value", [["parent_preset_1", "parent_preset_2"], "parent_preset_3"]
+    )
+    def test_preset_multiple_parents(
+        self, config_file, output_options, youtube_video, preset_value
+    ):
         preset = Preset(
             config=config_file,
             name="test",
             value={
-                "preset": ["parent_preset_1", "parent_preset_2"],
+                "preset": preset_value,
                 "youtube": youtube_video,
                 "output_options": output_options,
                 "nfo_tags": {"tags": {"key-3": "this-preset"}},