[REFACTOR] Single download options (#668)

* [REFACTOR] Single download options

* update

* fix unit test msg

src/ytdl_sub/config/preset_class_mappings.py

@@ -4,8 +4,7 @@ from typing import Type
 
 from ytdl_sub.config.plugin import Plugin
 from ytdl_sub.downloaders.source_plugin import SourcePlugin
-from ytdl_sub.downloaders.url.multi_url import MultiUrlDownloader
+from ytdl_sub.downloaders.url.downloader import MultiUrlDownloader
-from ytdl_sub.downloaders.url.url import UrlDownloader
 from ytdl_sub.plugins.audio_extract import AudioExtractPlugin
 from ytdl_sub.plugins.chapters import ChaptersPlugin
 from ytdl_sub.plugins.date_range import DateRangePlugin
@@ -30,7 +29,7 @@ class DownloadStrategyMapping:
     _MAPPING: Dict[str, Dict[str, Type[SourcePlugin]]] = {
         "download": {
             "multi_url": MultiUrlDownloader,
-            "url": UrlDownloader,
+            "url": MultiUrlDownloader,
         },
     }
 

src/ytdl_sub/downloaders/url/downloader.py

@@ -16,12 +16,6 @@ from ytdl_sub.config.plugin import PluginPriority
 from ytdl_sub.config.preset_options import Overrides
 from ytdl_sub.downloaders.source_plugin import SourcePlugin
 from ytdl_sub.downloaders.source_plugin import SourcePluginExtension
-from ytdl_sub.downloaders.url.multi_url_source_options_validator import (
-    MultiUrlSourceOptionsValidator,
-)
-from ytdl_sub.downloaders.url.multi_url_source_options_validator import (
-    TMultiURLSourceOptionsValidator,
-)
 from ytdl_sub.downloaders.url.validators import MultiUrlValidator
 from ytdl_sub.downloaders.url.validators import UrlThumbnailListValidator
 from ytdl_sub.downloaders.url.validators import UrlValidator
@@ -59,7 +53,7 @@ class UrlDownloaderThumbnailPlugin(SourcePluginExtension):
 
     def __init__(
         self,
-        options: MultiUrlSourceOptionsValidator,
+        options: MultiUrlValidator,
         overrides: Overrides,
         enhanced_download_archive: EnhancedDownloadArchive,
     ):
@@ -71,7 +65,7 @@ class UrlDownloaderThumbnailPlugin(SourcePluginExtension):
         self._thumbnails_downloaded: Set[str] = set()
         self._collection_url_mapping: Dict[str, UrlValidator] = {
             self.overrides.apply_formatter(collection_url.url): collection_url
-            for collection_url in options.collection_validator.urls.list
+            for collection_url in options.urls.list
         }
 
     def _download_parent_thumbnails(
@@ -160,7 +154,7 @@ class UrlDownloaderCollectionVariablePlugin(SourcePluginExtension):
 
     def __init__(
         self,
-        options: MultiUrlSourceOptionsValidator,
+        options: MultiUrlValidator,
         overrides: Overrides,
         enhanced_download_archive: EnhancedDownloadArchive,
     ):
@@ -172,7 +166,7 @@ class UrlDownloaderCollectionVariablePlugin(SourcePluginExtension):
         self._thumbnails_downloaded: Set[str] = set()
         self._collection_url_mapping: Dict[str, UrlValidator] = {
             self.overrides.apply_formatter(collection_url.url): collection_url
-            for collection_url in options.collection_validator.urls.list
+            for collection_url in options.urls.list
         }
 
     def modify_entry_metadata(self, entry: Entry) -> Optional[Entry]:
@@ -194,12 +188,13 @@ class UrlDownloaderCollectionVariablePlugin(SourcePluginExtension):
         return entry
 
 
-class BaseUrlDownloader(SourcePlugin[TMultiURLSourceOptionsValidator], ABC):
+class MultiUrlDownloader(SourcePlugin[MultiUrlValidator], ABC):
     """
     Class that interacts with ytdl to perform the download of metadata and content,
     and should translate that to list of Entry objects.
     """
 
+    plugin_options_type = MultiUrlValidator
     plugin_extensions = [UrlDownloaderThumbnailPlugin, UrlDownloaderCollectionVariablePlugin]
 
     @classmethod
@@ -214,7 +209,7 @@ class BaseUrlDownloader(SourcePlugin[TMultiURLSourceOptionsValidator], ABC):
 
     def __init__(
         self,
-        options: TMultiURLSourceOptionsValidator,
+        options: MultiUrlValidator,
         enhanced_download_archive: EnhancedDownloadArchive,
         download_ytdl_options: YTDLOptionsBuilder,
         metadata_ytdl_options: YTDLOptionsBuilder,
@@ -300,7 +295,7 @@ class BaseUrlDownloader(SourcePlugin[TMultiURLSourceOptionsValidator], ABC):
     @property
     def collection(self) -> MultiUrlValidator:
         """Return the download options collection"""
-        return self.plugin_options.collection_validator
+        return self.plugin_options
 
     @contextlib.contextmanager
     def _separate_download_archives(self, clear_info_json_files: bool = False):

src/ytdl_sub/downloaders/url/multi_url.py

@@ -1,14 +1,8 @@
-from typing import Dict
-from typing import List
-
-from ytdl_sub.downloaders.url.downloader import BaseUrlDownloader
-from ytdl_sub.downloaders.url.multi_url_source_options_validator import (
-    MultiUrlSourceOptionsValidator,
-)
 from ytdl_sub.downloaders.url.validators import MultiUrlValidator
 
 
-class MultiUrlDownloadOptions(MultiUrlValidator, MultiUrlSourceOptionsValidator):
+# TODO: Remove later - keep for docstring
+class MultiUrlDownloadOptions(MultiUrlValidator):
     """
     Downloads from multiple URLs. If an entry is returned from more than one URL, it will
     resolve to the bottom-most URL settings.
@@ -42,29 +36,3 @@ class MultiUrlDownloadOptions(MultiUrlValidator, MultiUrlSourceOptionsValidator)
                   - name: "season{season_index}-poster.jpg"
                     uid: "latest_entry"
     """
-
-    @property
-    def collection_validator(self) -> MultiUrlValidator:
-        """Returns itself!"""
-        return self
-
-    def validate_with_variables(
-        self, source_variables: List[str], override_variables: Dict[str, str]
-    ) -> None:
-        """
-        Validates any source variables added by the collection
-        """
-        super().validate_with_variables(
-            source_variables=source_variables, override_variables=override_variables
-        )
-
-        has_non_empty_url = False
-        for url_validator in self.urls.list:
-            has_non_empty_url |= bool(url_validator.url.apply_formatter(override_variables))
-
-        if not has_non_empty_url:
-            raise self._validation_exception("Must contain at least one url that is non-empty")
-
-
-class MultiUrlDownloader(BaseUrlDownloader[MultiUrlDownloadOptions]):
-    plugin_options_type = MultiUrlDownloadOptions

src/ytdl_sub/downloaders/url/multi_url_source_options_validator.py

@@ -1,47 +0,0 @@
-import abc
-from abc import ABC
-from typing import Dict
-from typing import List
-from typing import TypeVar
-
-from ytdl_sub.config.preset_options import OptionsValidator
-from ytdl_sub.downloaders.url.validators import MultiUrlValidator
-
-
-class MultiUrlSourceOptionsValidator(OptionsValidator, ABC):
-    """
-    Placeholder class to define downloader options
-    """
-
-    @property
-    @abc.abstractmethod
-    def collection_validator(self) -> MultiUrlValidator:
-        """
-        Returns
-        -------
-        MultiUrlValidator
-            To determine how the entries are downloaded
-        """
-
-    def added_source_variables(self) -> List[str]:
-        """
-        Returns
-        -------
-        Added source variables on the collection
-        """
-        return self.collection_validator.added_source_variables()
-
-    def validate_with_variables(
-        self, source_variables: List[str], override_variables: Dict[str, str]
-    ) -> None:
-        """
-        Validates any source variables added by the collection
-        """
-        self.collection_validator.validate_with_variables(
-            source_variables=source_variables, override_variables=override_variables
-        )
-
-
-TMultiURLSourceOptionsValidator = TypeVar(
-    "TMultiURLSourceOptionsValidator", bound=MultiUrlSourceOptionsValidator
-)

src/ytdl_sub/downloaders/url/url.py

@@ -1,12 +1,8 @@
-from ytdl_sub.downloaders.url.downloader import BaseUrlDownloader
-from ytdl_sub.downloaders.url.multi_url_source_options_validator import (
-    MultiUrlSourceOptionsValidator,
-)
-from ytdl_sub.downloaders.url.validators import MultiUrlValidator
 from ytdl_sub.downloaders.url.validators import UrlValidator
 
 
-class UrlDownloadOptions(UrlValidator, MultiUrlSourceOptionsValidator):
+# TODO: Remove later - keep for docstring
+class UrlDownloadOptions(UrlValidator):
     """
     Downloads from a single URL supported by yt-dlp.
 
@@ -28,15 +24,3 @@ class UrlDownloadOptions(UrlValidator, MultiUrlSourceOptionsValidator):
                 uid: "banner_uncropped"
             download_reverse: True
     """
-
-    @property
-    def collection_validator(self) -> MultiUrlValidator:
-        """Returns itself!"""
-        return MultiUrlValidator(
-            name=self._name,
-            value={"urls": [self._value]},
-        )
-
-
-class UrlDownloader(BaseUrlDownloader[UrlDownloadOptions]):
-    plugin_options_type = UrlDownloadOptions

src/ytdl_sub/downloaders/url/validators.py

@@ -1,9 +1,10 @@
+import copy
 from typing import Any
 from typing import Dict
 from typing import List
 from typing import Optional
 
-from ytdl_sub.config.preset_options import OptionsDictValidator
+from ytdl_sub.config.preset_options import OptionsValidator
 from ytdl_sub.validators.strict_dict_validator import StrictDictValidator
 from ytdl_sub.validators.string_formatter_validators import DictFormatterValidator
 from ytdl_sub.validators.string_formatter_validators import OverridesStringFormatterValidator
@@ -172,26 +173,41 @@ class UrlListValidator(ListValidator[UrlValidator]):
                     collection_variables[var] = added_variables[var]
 
 
-class MultiUrlValidator(OptionsDictValidator):
+class MultiUrlValidator(OptionsValidator):
     """
     Downloads from multiple URLs. If an entry is returned from more than one URL, it will
     resolve to the bottom-most URL settings.
     """
 
-    _required_keys = {"urls"}
-
     @classmethod
     def partial_validate(cls, name: str, value: Any) -> None:
         """
         Partially validate a collection
         """
         if isinstance(value, dict):
-            value["urls"] = value.get("urls", [{"url": "placeholder"}])
+            value["url"] = value.get("url", "sadfasdf")
         _ = cls(name, value)
 
     def __init__(self, name, value):
         super().__init__(name, value)
-        self._urls = self._validate_key(key="urls", validator=UrlListValidator)
+
+        # Copy since we're popping things
+        value_copy = copy.deepcopy(value)
+        if isinstance(value, dict):
+            # Pop old required field in case it's still there
+            value_copy.pop("download_strategy", None)
+
+            if "urls" in value_copy:
+                self._urls = UrlListValidator(name=name, value=value_copy["urls"])
+            else:
+                # Validate using a single URL validator first
+                _ = UrlValidator(name=name, value=value_copy)
+                self._urls = UrlListValidator(name=name, value=[value_copy])
+        else:
+            # Should error here. TODO: Add simplifications download here (string, list)
+            self._urls = UrlListValidator(
+                name=name, value=UrlValidator(name=name, value=value_copy)
+            )
 
     @property
     def urls(self) -> UrlListValidator:
@@ -248,3 +264,11 @@ class MultiUrlValidator(OptionsDictValidator):
                 _ = StringFormatterValidator(
                     name=f"{self._name}.{source_var_name}", value=source_var_formatter_str
                 ).apply_formatter(base_variables)
+
+        # Ensure at least URL is non-empty
+        has_non_empty_url = False
+        for url_validator in self.urls.list:
+            has_non_empty_url |= bool(url_validator.url.apply_formatter(override_variables))
+
+        if not has_non_empty_url:
+            raise self._validation_exception("Must contain at least one url that is non-empty")

tests/unit/config/test_config_file.py

@@ -98,7 +98,8 @@ class TestConfigFilePartiallyValidatesPresets:
             preset_dict={"download": {"download_strategy": "multi_url", "bad_key": "nope"}},
             expected_error_message="Validation error in partial_preset.download: "
             "'partial_preset.download' contains the field 'bad_key' which is not allowed. "
-            "Allowed fields: urls",
+            "Allowed fields: download_reverse, playlist_thumbnails, source_thumbnails, url, "
+            "variables",
         )
 
     @pytest.mark.parametrize(

tests/unit/config/test_preset.py

@@ -3,7 +3,6 @@ import re
 import pytest
 
 from ytdl_sub.config.preset import Preset
-from ytdl_sub.downloaders.url.multi_url import MultiUrlDownloadOptions
 from ytdl_sub.plugins.nfo_tags import NfoTagsOptions
 from ytdl_sub.utils.exceptions import StringFormattingVariableNotFoundException
 from ytdl_sub.utils.exceptions import ValidationException

tests/unit/prebuilt_presets/conftest.py

@@ -10,7 +10,7 @@ import pytest
 from resources import copy_file_fixture
 
 from ytdl_sub.config.config_file import ConfigFile
-from ytdl_sub.downloaders.url.downloader import BaseUrlDownloader
+from ytdl_sub.downloaders.url.downloader import MultiUrlDownloader
 from ytdl_sub.downloaders.ytdlp import YTDLP
 from ytdl_sub.entries.variables.kwargs import DESCRIPTION
 from ytdl_sub.entries.variables.kwargs import EPOCH
@@ -201,7 +201,7 @@ def mock_download_collection_entries(
         with patch.object(
             YTDLP, "extract_info_via_info_json", new=_write_entries_to_working_dir
         ), patch.object(
-            BaseUrlDownloader, "_extract_entry_info_with_retry", new=lambda _, entry: entry
+            MultiUrlDownloader, "_extract_entry_info_with_retry", new=lambda _, entry: entry
         ):
             # Stub out metadata. TODO: update this if we do metadata plugins
             yield